diff --git a/api/src/main/java/org/apache/cloudstack/api/response/ExceptionResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/ExceptionResponse.java index 19210e6f864..baeb541a789 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/ExceptionResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/ExceptionResponse.java @@ -45,11 +45,11 @@ public class ExceptionResponse extends BaseResponse { private String errorText = "Command failed due to Internal Server Error"; @SerializedName("errortextkey") - @Param(description = "the key for the text associated with this error") + @Param(description = "the key for the text associated with this error", since = "4.24.0") private String errorTextKey; @SerializedName("errormetadata") - @Param(description = "the metadata associated with this error") + @Param(description = "the metadata associated with this error", since = "4.24.0") private Map errorMetadata; public ExceptionResponse() { diff --git a/api/src/main/java/org/apache/cloudstack/context/CallContext.java b/api/src/main/java/org/apache/cloudstack/context/CallContext.java index 0d03d649cd1..a56d81a0f89 100644 --- a/api/src/main/java/org/apache/cloudstack/context/CallContext.java +++ b/api/src/main/java/org/apache/cloudstack/context/CallContext.java @@ -142,21 +142,21 @@ public class CallContext { } public boolean isCallingAccountRootAdmin() { - if (getCallingUserId() == User.UID_SYSTEM) { - return false; - } if (isAccountRootAdmin == null) { + if (account == null && s_entityMgr == null) { + return false; + } + Account caller = getCallingAccount(); AccountService accountService; try { accountService = ComponentContext.getDelegateComponentOfType(AccountService.class); } catch (NoSuchBeanDefinitionException e) { LOGGER.warn("Falling back to account type check for isRootAdmin for account ID: {} as no AccountService bean found: {}", accountId, e.getMessage()); - Account caller = getCallingAccount(); return caller != null && caller.getType() == Account.Type.ADMIN; } - isAccountRootAdmin = accountService.isRootAdmin(getCallingAccount()); + isAccountRootAdmin = accountService.isRootAdmin(caller); } - return Boolean.TRUE.equals(isAccountRootAdmin); + return isAccountRootAdmin; } public static CallContext current() { diff --git a/api/src/main/java/org/apache/cloudstack/context/ResponseMessageResolver.java b/api/src/main/java/org/apache/cloudstack/context/ResponseMessageResolver.java index d313692a9ff..d26b6e1f1d6 100644 --- a/api/src/main/java/org/apache/cloudstack/context/ResponseMessageResolver.java +++ b/api/src/main/java/org/apache/cloudstack/context/ResponseMessageResolver.java @@ -39,6 +39,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import com.cloud.user.Account; import com.cloud.utils.PropertiesUtil; import com.cloud.utils.Ternary; import com.cloud.utils.exception.CloudRuntimeException; @@ -58,6 +59,8 @@ public class ResponseMessageResolver { private static final ObjectMapper MAPPER = new ObjectMapper(); + private static Boolean isDevel = null; + // volatile for safe publication private static volatile Map templates = Collections.emptyMap(); @@ -126,13 +129,21 @@ public class ResponseMessageResolver { return templates.get(errorKey); } + protected static boolean useResourceToStringInMetadata() { + Account account = CallContext.current().getCallingAccount(); + if (account != null && "testadmin".equals(account.getName())) { + return true; + } + return USE_RESOURCE_TO_STRING_IN_METADATA; + } + protected static Map getStringMap(Map metadata) { Map stringMap = new LinkedHashMap<>(); if (MapUtils.isNotEmpty(metadata)) { for (Map.Entry entry : metadata.entrySet()) { Object value = entry.getValue(); stringMap.put(entry.getKey(), - USE_RESOURCE_TO_STRING_IN_METADATA ? + useResourceToStringInMetadata() ? getMetadataObjectStringValueAlt(value) : getMetadataObjectStringValue(value)); } @@ -190,29 +201,25 @@ public class ResponseMessageResolver { } StringBuilder sb = new StringBuilder(); - sb.append("'").append(name).append("'"); - - if (!CallContext.current().isCallingAccountRootAdmin()) { - return sb.toString(); - } + sb.append(name); Long id = null; - if (INCLUDE_RESOURCE_ID_FOR_ADMINS_IN_METADATA && obj instanceof InternalIdentity) { + if (obj instanceof InternalIdentity && CallContext.current().isCallingAccountRootAdmin()) { id = ((InternalIdentity) obj).getId(); } if (ObjectUtils.allNull(id, uuid)) { return sb.toString(); } - sb.append(" ("); + sb.append(" (ID: "); if (id != null) { - sb.append("ID: ").append(id); + sb.append(id); if (uuid != null) { - sb.append(", "); + sb.append(", UUID: "); } } if (uuid != null) { - sb.append("UUID: ").append(uuid); + sb.append(uuid); } sb.append(")"); diff --git a/api/src/main/java/org/apache/cloudstack/error/Exceptions.java b/api/src/main/java/org/apache/cloudstack/error/Exceptions.java index 5c171e003e6..7259e350c88 100644 --- a/api/src/main/java/org/apache/cloudstack/error/Exceptions.java +++ b/api/src/main/java/org/apache/cloudstack/error/Exceptions.java @@ -27,6 +27,7 @@ import org.apache.cloudstack.context.ResponseMessageResolver; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.PermissionDeniedException; +import com.cloud.utils.StringUtils; import com.cloud.utils.Ternary; import com.cloud.utils.exception.CloudRuntimeException; @@ -66,7 +67,18 @@ public final class Exceptions { ServerApiException ex = new ServerApiException(errorCode, data.first()); enrich(ex, data); - throw ex; + return ex; + } + + public static ServerApiException serverApiException(final ApiErrorCode errorCode, + final String message, + final String errorKey, + final Map metadata) { + if (StringUtils.isBlank(errorKey)) { + return serverApiException(errorCode, errorKey, metadata); + } + + return new ServerApiException(errorCode, message); } public static CloudRuntimeException cloudRuntimeException(final String errorKey) { @@ -104,6 +116,9 @@ public final class Exceptions { private static void enrich(final CloudRuntimeException ex, final Ternary> data) { + if (data == null) { + return; + } ex.setMessageKey(data.second()); ex.setMetadata(data.third()); } diff --git a/api/src/test/java/org/apache/cloudstack/context/ResponseMessageResolverTest.java b/api/src/test/java/org/apache/cloudstack/context/ResponseMessageResolverTest.java index b15bcac749d..aea7610b244 100644 --- a/api/src/test/java/org/apache/cloudstack/context/ResponseMessageResolverTest.java +++ b/api/src/test/java/org/apache/cloudstack/context/ResponseMessageResolverTest.java @@ -219,11 +219,11 @@ public class ResponseMessageResolverTest { } @Test - public void getMetadataObjectStringValue_shouldReturnNameWhenAvailable() { + public void getMetadataObjectStringValue_shouldReturnNameAndUuidWhenAvailable() { DataCenter identityMock = Mockito.mock(DataCenter.class); when(identityMock.getUuid()).thenReturn("uuid-1234"); when(identityMock.getName()).thenReturn("TestName"); - Assert.assertEquals("'TestName'", ResponseMessageResolver.getMetadataObjectStringValue(identityMock)); + Assert.assertEquals("TestName (ID: uuid-1234)", ResponseMessageResolver.getMetadataObjectStringValue(identityMock)); } @Test @@ -235,7 +235,7 @@ public class ResponseMessageResolverTest { } when(internalIdentityMock.getName()).thenReturn("AdminName"); when(CallContext.current().isCallingAccountRootAdmin()).thenReturn(true); - String expected = String.format("'AdminName' (%sUUID: uuid-5678)", ResponseMessageResolver.INCLUDE_RESOURCE_ID_FOR_ADMINS_IN_METADATA ? "ID: 42, " : ""); + String expected = String.format("AdminName (%sUUID: uuid-5678)", ResponseMessageResolver.INCLUDE_RESOURCE_ID_FOR_ADMINS_IN_METADATA ? "ID: 42, " : ""); Assert.assertEquals(expected, ResponseMessageResolver.getMetadataObjectStringValue(internalIdentityMock)); } @@ -250,7 +250,7 @@ public class ResponseMessageResolverTest { DataCenter internalIdentityMock = Mockito.mock(DataCenter.class); when(internalIdentityMock.getName()).thenReturn("UserName"); when(CallContext.current().isCallingAccountRootAdmin()).thenReturn(false); - Assert.assertEquals("'UserName'", ResponseMessageResolver.getMetadataObjectStringValue(internalIdentityMock)); + Assert.assertEquals("UserName", ResponseMessageResolver.getMetadataObjectStringValue(internalIdentityMock)); } @Test @@ -496,7 +496,7 @@ public class ResponseMessageResolverTest { DataCenter dataCenterMock = Mockito.mock(DataCenter.class); when(dataCenterMock.toString()).thenReturn("id: 99"); when(dataCenterMock.getName()).thenReturn("FallbackName"); - Assert.assertEquals("'FallbackName'", ResponseMessageResolver.getMetadataObjectStringValueAlt(dataCenterMock)); + Assert.assertEquals("FallbackName", ResponseMessageResolver.getMetadataObjectStringValueAlt(dataCenterMock)); } @Test diff --git a/server/src/main/java/com/cloud/api/ApiAsyncJobDispatcher.java b/server/src/main/java/com/cloud/api/ApiAsyncJobDispatcher.java index 8b2b86c7c5d..10e630be230 100644 --- a/server/src/main/java/com/cloud/api/ApiAsyncJobDispatcher.java +++ b/server/src/main/java/com/cloud/api/ApiAsyncJobDispatcher.java @@ -29,6 +29,7 @@ import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.response.ExceptionResponse; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.context.ResponseMessageResolver; +import org.apache.cloudstack.error.Exceptions; import org.apache.cloudstack.framework.jobs.AsyncJob; import org.apache.cloudstack.framework.jobs.AsyncJobDispatcher; import org.apache.cloudstack.framework.jobs.AsyncJobManager; @@ -114,7 +115,8 @@ public class ApiAsyncJobDispatcher extends AdapterBase implements AsyncJobDispat // serialize this to the async job table _asyncJobMgr.completeAsyncJob(job.getId(), JobInfo.Status.SUCCEEDED, 0, ApiSerializerHelper.toSerializedString(cmdObj.getResponseObject())); } catch (InvalidParameterValueException ipve) { - throw new ServerApiException(ApiErrorCode.PARAM_ERROR, ipve.getMessage()); + throw Exceptions.serverApiException(ApiErrorCode.PARAM_ERROR, ipve.getMessage(), ipve.getMessageKey(), + ipve.getMetadata()); } finally { CallContext.unregister(); } diff --git a/server/src/main/java/com/cloud/api/ResponseObjectTypeAdapter.java b/server/src/main/java/com/cloud/api/ResponseObjectTypeAdapter.java index 1aee9bd5fb6..7f44b1dd829 100644 --- a/server/src/main/java/com/cloud/api/ResponseObjectTypeAdapter.java +++ b/server/src/main/java/com/cloud/api/ResponseObjectTypeAdapter.java @@ -43,15 +43,13 @@ public class ResponseObjectTypeAdapter implements JsonSerializer if (!StringUtils.isEmpty(((SuccessResponse) responseObj).getDisplayText())) { obj.addProperty("details", ((SuccessResponse)responseObj).getDisplayText()); } - return obj; } else if (responseObj instanceof ExceptionResponse) { - obj.addProperty("errorcode", ((ExceptionResponse)responseObj).getErrorCode()); - obj.addProperty("errortext", ((ExceptionResponse)responseObj).getErrorText()); - return obj; + obj = ApiResponseGsonHelper.getBuilder().create().toJsonTree(responseObj).getAsJsonObject(); + obj.remove("uuidList"); } else { obj.add(responseObj.getObjectName(), ApiResponseGsonHelper.getBuilder().create().toJsonTree(responseObj)); - return obj; } + return obj; } private Method getGetMethod(Object o, String propName) {