Skip to content

Cleanup APIs getCommandName #7022

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DaanHoogland merged 4 commits into from
Jan 3, 2023
Merged

Cleanup APIs getCommandName #7022

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
42a8326
add instructions to getCommandName method on BaseCmd
Dec 23, 2022
6418b42
remove unnecessary overrides, imports and apiname/responsename declar…
Dec 26, 2022
e656d27
apply changes to BaseCmd methods
Dec 27, 2022
4dc1a7a
removing unused variables (Name/NAME) pointed out by code smells
Dec 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
38 changes: 22 additions & 16 deletions api/src/main/java/org/apache/cloudstack/api/BaseCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,30 +247,36 @@ public void setResponseType(final String responseType) {
this.responseType = responseType;
}

/**
* For some reason this method does not return the actual command name, but more a name that
* is used to create the response. So you can expect for a XCmd a value like xcmdresponse. Anyways
* this methods is used in too many places so for now instead of changing it we just create another
* method {@link BaseCmd#getActualCommandName()} that returns the value from {@link APICommand#name()}
*
* @return
*/
public abstract String getCommandName();


/**
* Gets the CommandName based on the class annotations: the value from {@link APICommand#name()}
*
* @return the value from {@link APICommand#name()}
*/
public String getActualCommandName() {
public static String getCommandNameByClass(Class<?> clazz) {
Comment thread
ezntt marked this conversation as resolved.
String cmdName = null;
if (this.getClass().getAnnotation(APICommand.class) != null) {
cmdName = this.getClass().getAnnotation(APICommand.class).name();
APICommand apiClassAnnotation = clazz.getAnnotation(APICommand.class);

if (apiClassAnnotation != null && apiClassAnnotation.name() != null) {
cmdName = apiClassAnnotation.name();
} else {
cmdName = this.getClass().getName();
cmdName = clazz.getName();
Comment thread
weizhouapache marked this conversation as resolved.
}
return cmdName;
return cmdName;
}

public String getActualCommandName() {
return getCommandNameByClass(this.getClass());
}

public String getCommandName() {
return getResponseNameByClass(this.getClass());
}

/**
* Retrieves the name defined in {@link APICommand#name()}, in lower case, with the prefix {@link BaseCmd#RESPONSE_SUFFIX}
*/
public static String getResponseNameByClass(Class<?> clazz) {

@DaanHoogland DaanHoogland Dec 27, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this being used?

@ezntt ezntt Dec 27, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ezntt , good initiative. What do you propose we do with the remaining 88?

As the PR is intended to clean up the repeated code (it will not change the APIs behavior), I would rather not touch those 88 APIs here. My proposal for those remaining 88 APIs is a later discussion on how we should handle them and if changing them would affect any other place.

is this being used?

Yes, it is used in class UploadListener.java:

    public static final Map<String, String> responseNameMap;
    static {
        Map<String, String> tempMap = new HashMap<String, String>();
        tempMap.put(Type.ISO.toString(), BaseCmd.getResponseNameByClass(ExtractIsoCmd.class));
        tempMap.put(Type.TEMPLATE.toString(), BaseCmd.getResponseNameByClass(ExtractTemplateCmd.class));
        tempMap.put(Type.VOLUME.toString(), BaseCmd.getResponseNameByClass(ExtractVolumeCmd.class));
        tempMap.put("DEFAULT", "extractresponse");
        responseNameMap = Collections.unmodifiableMap(tempMap);
    }

return getCommandNameByClass(clazz).toLowerCase() + BaseCmd.RESPONSE_SUFFIX;
}

/**
Expand Down
6 changes: 0 additions & 6 deletions api/src/main/java/org/apache/cloudstack/api/command/admin/account/CreateAccountCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
public class CreateAccountCmd extends BaseCmd {
public static final Logger s_logger = Logger.getLogger(CreateAccountCmd.class.getName());

private static final String s_name = "createaccountresponse";

/////////////////////////////////////////////////////
//////////////// API parameters /////////////////////
Expand Down Expand Up @@ -172,11 +171,6 @@ public String getUserUUID() {
/////////////// API Implementation///////////////////
/////////////////////////////////////////////////////

@Override
public String getCommandName() {
return s_name;
}

@Override
public long getEntityOwnerId() {
return Account.ACCOUNT_ID_SYSTEM;
Expand Down
10 changes: 0 additions & 10 deletions api/src/main/java/org/apache/cloudstack/api/command/admin/account/DeleteAccountCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
public class DeleteAccountCmd extends BaseAsyncCmd {
public static final Logger s_logger = Logger.getLogger(DeleteAccountCmd.class.getName());
private static final String s_name = "deleteaccountresponse";

/////////////////////////////////////////////////////
//////////////// API parameters /////////////////////
Expand All @@ -64,15 +63,6 @@ public Long getId() {
/////////////// API Implementation///////////////////
/////////////////////////////////////////////////////

public static String getStaticName() {
return s_name;
}

@Override
public String getCommandName() {
return s_name;
}

@Override
public long getEntityOwnerId() {
Account account = CallContext.current().getCallingAccount();// Let's give the caller here for event logging.
Expand Down
6 changes: 0 additions & 6 deletions api/src/main/java/org/apache/cloudstack/api/command/admin/account/DisableAccountCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
requestHasSensitiveInfo = false, responseHasSensitiveInfo = true)
public class DisableAccountCmd extends BaseAsyncCmd {
public static final Logger s_logger = Logger.getLogger(DisableAccountCmd.class.getName());
private static final String s_name = "disableaccountresponse";

/////////////////////////////////////////////////////
//////////////// API parameters /////////////////////
Expand Down Expand Up @@ -89,11 +88,6 @@ public Boolean getLockRequested() {
/////////////// API Implementation///////////////////
/////////////////////////////////////////////////////

@Override
public String getCommandName() {
return s_name;
}

@Override
public String getEventType() {
return EventTypes.EVENT_ACCOUNT_DISABLE;
Expand Down
6 changes: 0 additions & 6 deletions api/src/main/java/org/apache/cloudstack/api/command/admin/account/EnableAccountCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
requestHasSensitiveInfo = false, responseHasSensitiveInfo = true)
public class EnableAccountCmd extends BaseCmd {
public static final Logger s_logger = Logger.getLogger(EnableAccountCmd.class.getName());
private static final String s_name = "enableaccountresponse";

/////////////////////////////////////////////////////
//////////////// API parameters /////////////////////
Expand Down Expand Up @@ -78,11 +77,6 @@ public Long getDomainId() {
/////////////// API Implementation///////////////////
/////////////////////////////////////////////////////

@Override
public String getCommandName() {
return s_name;
}

@Override
public long getEntityOwnerId() {
Account account = _entityMgr.findById(Account.class, getId());
Expand Down
6 changes: 0 additions & 6 deletions api/src/main/java/org/apache/cloudstack/api/command/admin/account/LockAccountCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
public class LockAccountCmd extends BaseCmd {
public static final Logger s_logger = Logger.getLogger(LockAccountCmd.class.getName());

private static final String s_name = "lockaccountresponse";

/////////////////////////////////////////////////////
//////////////// API parameters /////////////////////
Expand Down Expand Up @@ -69,11 +68,6 @@ public Long getDomainId() {
/////////////// API Implementation///////////////////
/////////////////////////////////////////////////////

@Override
public String getCommandName() {
return s_name;
}

@Override
public long getEntityOwnerId() {
final Account account = _accountService.getActiveAccountByName(getAccountName(), getDomainId());
Expand Down
6 changes: 0 additions & 6 deletions api/src/main/java/org/apache/cloudstack/api/command/admin/account/UpdateAccountCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
requestHasSensitiveInfo = false, responseHasSensitiveInfo = true)
public class UpdateAccountCmd extends BaseCmd {
public static final Logger s_logger = Logger.getLogger(UpdateAccountCmd.class.getName());
private static final String s_name = "updateaccountresponse";

/////////////////////////////////////////////////////
//////////////// API parameters /////////////////////
Expand Down Expand Up @@ -116,11 +115,6 @@ public Map getDetails() {
/////////////// API Implementation///////////////////
/////////////////////////////////////////////////////

@Override
public String getCommandName() {
return s_name;
}

@Override
public long getEntityOwnerId() {
Account account = _entityMgr.findById(Account.class, getId());
Expand Down
9 changes: 1 addition & 8 deletions api/src/main/java/org/apache/cloudstack/api/command/admin/acl/CreateRoleCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,18 @@
import org.apache.cloudstack.api.ApiCommandResourceType;
import org.apache.cloudstack.api.ApiConstants;
import org.apache.cloudstack.api.ApiErrorCode;
import org.apache.cloudstack.api.BaseCmd;
import org.apache.cloudstack.api.Parameter;
import org.apache.cloudstack.api.ServerApiException;
import org.apache.cloudstack.api.response.RoleResponse;
import org.apache.cloudstack.context.CallContext;

import com.cloud.user.Account;

@APICommand(name = CreateRoleCmd.APINAME, description = "Creates a role", responseObject = RoleResponse.class,
@APICommand(name = "createRole", description = "Creates a role", responseObject = RoleResponse.class,
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
since = "4.9.0",
authorized = {RoleType.Admin})
public class CreateRoleCmd extends RoleCmd {
public static final String APINAME = "createRole";

/////////////////////////////////////////////////////
//////////////// API parameters /////////////////////
Expand Down Expand Up @@ -66,11 +64,6 @@ public Long getRoleId() {
/////////////// API Implementation///////////////////
/////////////////////////////////////////////////////

@Override
public String getCommandName() {
return APINAME.toLowerCase() + BaseCmd.RESPONSE_SUFFIX;
}

@Override
public long getEntityOwnerId() {
return Account.ACCOUNT_ID_SYSTEM;
Expand Down
9 changes: 1 addition & 8 deletions api/src/main/java/org/apache/cloudstack/api/command/admin/acl/CreateRolePermissionCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import org.apache.cloudstack.api.ApiArgValidator;
import org.apache.cloudstack.api.ApiConstants;
import org.apache.cloudstack.api.ApiErrorCode;
import org.apache.cloudstack.api.BaseCmd;
import org.apache.cloudstack.api.Parameter;
import org.apache.cloudstack.api.ServerApiException;
import org.apache.cloudstack.api.response.RolePermissionResponse;
Expand All @@ -33,12 +32,11 @@

import com.cloud.user.Account;

@APICommand(name = CreateRolePermissionCmd.APINAME, description = "Adds an API permission to a role", responseObject = RolePermissionResponse.class,
@APICommand(name = "createRolePermission", description = "Adds an API permission to a role", responseObject = RolePermissionResponse.class,
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
since = "4.9.0",
authorized = {RoleType.Admin})
public class CreateRolePermissionCmd extends BaseRolePermissionCmd {
public static final String APINAME = "createRolePermission";

/////////////////////////////////////////////////////
//////////////// API parameters /////////////////////
Expand All @@ -60,11 +58,6 @@ public Long getRoleId() {
/////////////// API Implementation///////////////////
/////////////////////////////////////////////////////

@Override
public String getCommandName() {
return APINAME.toLowerCase() + BaseCmd.RESPONSE_SUFFIX;
}

@Override
public long getEntityOwnerId() {
return Account.ACCOUNT_ID_SYSTEM;
Expand Down
8 changes: 1 addition & 7 deletions api/src/main/java/org/apache/cloudstack/api/command/admin/acl/DeleteRoleCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,11 @@

import com.cloud.user.Account;

@APICommand(name = DeleteRoleCmd.APINAME, description = "Deletes a role", responseObject = SuccessResponse.class,
@APICommand(name = "deleteRole", description = "Deletes a role", responseObject = SuccessResponse.class,
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
since = "4.9.0",
authorized = {RoleType.Admin})
public class DeleteRoleCmd extends BaseCmd {
public static final String APINAME = "deleteRole";

/////////////////////////////////////////////////////
//////////////// API parameters /////////////////////
Expand All @@ -60,11 +59,6 @@ public Long getRoleId() {
/////////////// API Implementation///////////////////
/////////////////////////////////////////////////////

@Override
public String getCommandName() {
return APINAME.toLowerCase() + BaseCmd.RESPONSE_SUFFIX;
}

@Override
public long getEntityOwnerId() {
return Account.ACCOUNT_ID_SYSTEM;
Expand Down
8 changes: 1 addition & 7 deletions api/src/main/java/org/apache/cloudstack/api/command/admin/acl/DeleteRolePermissionCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,11 @@
import org.apache.cloudstack.api.response.SuccessResponse;
import org.apache.cloudstack.context.CallContext;

@APICommand(name = DeleteRolePermissionCmd.APINAME, description = "Deletes a role permission", responseObject = SuccessResponse.class,
@APICommand(name = "deleteRolePermission", description = "Deletes a role permission", responseObject = SuccessResponse.class,
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
since = "4.9.0",
authorized = {RoleType.Admin})
public class DeleteRolePermissionCmd extends BaseCmd {
public static final String APINAME = "deleteRolePermission";

/////////////////////////////////////////////////////
//////////////// API parameters /////////////////////
Expand All @@ -58,11 +57,6 @@ public Long getRolePermissionId() {
/////////////// API Implementation///////////////////
/////////////////////////////////////////////////////

@Override
public String getCommandName() {
return APINAME.toLowerCase() + BaseCmd.RESPONSE_SUFFIX;
}

@Override
public long getEntityOwnerId() {
return Account.ACCOUNT_ID_SYSTEM;
Expand Down
9 changes: 1 addition & 8 deletions api/src/main/java/org/apache/cloudstack/api/command/admin/acl/ImportRoleCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import org.apache.cloudstack.api.ApiConstants;
import org.apache.cloudstack.api.ApiErrorCode;
import org.apache.cloudstack.api.ApiServerService;
import org.apache.cloudstack.api.BaseCmd;
import org.apache.cloudstack.api.Parameter;
import org.apache.cloudstack.api.ServerApiException;
import org.apache.cloudstack.api.response.RoleResponse;
Expand All @@ -43,12 +42,11 @@

import com.cloud.user.Account;

@APICommand(name = ImportRoleCmd.APINAME, description = "Imports a role based on provided map of rule permissions", responseObject = RoleResponse.class,
@APICommand(name = "importRole", description = "Imports a role based on provided map of rule permissions", responseObject = RoleResponse.class,
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
since = "4.15.0",
authorized = {RoleType.Admin})
public class ImportRoleCmd extends RoleCmd {
public static final String APINAME = "importRole";

/////////////////////////////////////////////////////
//////////////// API parameters /////////////////////
Expand Down Expand Up @@ -120,11 +118,6 @@ public boolean isForced() {
/////////////// API Implementation///////////////////
/////////////////////////////////////////////////////

@Override
public String getCommandName() {
return APINAME.toLowerCase() + BaseCmd.RESPONSE_SUFFIX;
}

@Override
public long getEntityOwnerId() {
return Account.ACCOUNT_ID_SYSTEM;
Expand Down
8 changes: 1 addition & 7 deletions api/src/main/java/org/apache/cloudstack/api/command/admin/acl/ListRolePermissionsCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,11 @@
import java.util.List;


@APICommand(name = ListRolePermissionsCmd.APINAME, description = "Lists role permissions", responseObject = RolePermissionResponse.class,
@APICommand(name = "listRolePermissions", description = "Lists role permissions", responseObject = RolePermissionResponse.class,
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
since = "4.9.0",
authorized = {RoleType.Admin})
public class ListRolePermissionsCmd extends BaseCmd {
public static final String APINAME = "listRolePermissions";

/////////////////////////////////////////////////////
//////////////// API parameters /////////////////////
Expand All @@ -64,11 +63,6 @@ public Long getRoleId() {
/////////////// API Implementation///////////////////
/////////////////////////////////////////////////////

@Override
public String getCommandName() {
return APINAME.toLowerCase() + BaseCmd.RESPONSE_SUFFIX;
}

@Override
public long getEntityOwnerId() {
return Account.ACCOUNT_ID_SYSTEM;
Expand Down
8 changes: 1 addition & 7 deletions api/src/main/java/org/apache/cloudstack/api/command/admin/acl/ListRolesCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,9 @@
import com.cloud.user.Account;
import com.cloud.utils.Pair;

@APICommand(name = ListRolesCmd.APINAME, description = "Lists dynamic roles in CloudStack", responseObject = RoleResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.9.0", authorized = {
@APICommand(name = "listRoles", description = "Lists dynamic roles in CloudStack", responseObject = RoleResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.9.0", authorized = {
RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin })
public class ListRolesCmd extends BaseListCmd {
public static final String APINAME = "listRoles";

/////////////////////////////////////////////////////
//////////////// API parameters /////////////////////
Expand Down Expand Up @@ -75,11 +74,6 @@ public RoleType getRoleType() {
/////////////// API Implementation///////////////////
/////////////////////////////////////////////////////

@Override
public String getCommandName() {
return APINAME.toLowerCase() + BaseListCmd.RESPONSE_SUFFIX;
}

@Override
public long getEntityOwnerId() {
return Account.ACCOUNT_ID_SYSTEM;
Expand Down
Loading