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 eee7890a3a4..19210e6f864 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 @@ -46,7 +46,7 @@ public class ExceptionResponse extends BaseResponse { @SerializedName("errortextkey") @Param(description = "the key for the text associated with this error") - private String errorTextKey = ""; + private String errorTextKey; @SerializedName("errormetadata") @Param(description = "the metadata associated with this error") diff --git a/api/src/main/java/org/apache/cloudstack/context/ErrorMessageResolver.java b/api/src/main/java/org/apache/cloudstack/context/ErrorMessageResolver.java index 8dbec96fb76..e9f30e2295d 100644 --- a/api/src/main/java/org/apache/cloudstack/context/ErrorMessageResolver.java +++ b/api/src/main/java/org/apache/cloudstack/context/ErrorMessageResolver.java @@ -22,9 +22,13 @@ import java.io.InputStream; import java.lang.reflect.InvocationTargetException; import java.nio.file.Files; import java.nio.file.Paths; +import java.util.ArrayList; import java.util.Collections; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import org.apache.cloudstack.api.Identity; import org.apache.cloudstack.api.InternalIdentity; @@ -41,14 +45,15 @@ import com.cloud.utils.exception.CloudRuntimeException; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; -public final class ErrorMessageResolver { - +public class ErrorMessageResolver { private static final Logger LOG = LogManager.getLogger(ErrorMessageResolver.class); - private static final String ERROR_MESSAGES_FILENAME = "error-messages.json"; - private static final String ERROR_KEY_ADMIN_SUFFIX = ".admin"; - private static final boolean INCLUDE_METADATA_ID_IN_MESSAGE = false; + protected static final String ERROR_MESSAGES_FILENAME = "error-messages.json"; + protected static final String ERROR_KEY_ADMIN_SUFFIX = ".admin"; + protected static final boolean INCLUDE_METADATA_ID_IN_MESSAGE = false; + + private static final Pattern VARIABLE_PATTERN = Pattern.compile("\\{\\{\\s*([A-Za-z0-9_]+)\\s*\\}\\}"); private static final ObjectMapper MAPPER = new ObjectMapper(); @@ -61,11 +66,43 @@ public final class ErrorMessageResolver { private ErrorMessageResolver() { } - public static String getMessage(String errorKey, Map metadata) { - return getMessageUsingStringMap(errorKey, getStringMap(metadata)); + protected static List getVariableNamesInErrorKey(String template) { + if (template == null || template.isEmpty()) { + return Collections.emptyList(); + } + List variables = new ArrayList<>(); + Matcher matcher = VARIABLE_PATTERN.matcher(template); + while (matcher.find()) { + String name = matcher.group(1); + if (name != null && !name.isEmpty()) { + variables.add(name); + } + } + return variables; } - private static String getTemplateForKey(String errorKey) { + protected static Map getCombinedMetadataFromErrorTemplate(String template, Map metadata) { + List variableNames = getVariableNamesInErrorKey(template); + if (variableNames.isEmpty()) { + return metadata; + } + Map contextMetadata = CallContext.current().getContextStringKeyParameters(); + if (MapUtils.isEmpty(contextMetadata)) { + return metadata; + } + Map combinedMetadata = new LinkedHashMap<>(); + for (String varName : variableNames) { + if (contextMetadata.containsKey(varName)) { + combinedMetadata.put(varName, contextMetadata.get(varName)); + } + } + if (MapUtils.isNotEmpty(metadata)) { + combinedMetadata.putAll(metadata); + } + return combinedMetadata; + } + + protected static String getTemplateForKey(String errorKey) { if (errorKey == null) { return null; } @@ -79,15 +116,7 @@ public final class ErrorMessageResolver { return templates.get(errorKey); } - private static String getMessageUsingStringMap(String errorKey, Map metadata) { - String template = getTemplateForKey(errorKey); - if (template == null) { - return errorKey; - } - return expand(template, metadata); - } - - private static Map getStringMap(Map metadata) { + protected static Map getStringMap(Map metadata) { Map stringMap = new LinkedHashMap<>(); if (MapUtils.isNotEmpty(metadata)) { for (Map.Entry entry : metadata.entrySet()) { @@ -98,16 +127,35 @@ public final class ErrorMessageResolver { return stringMap; } - private static String getMetadataObjectStringValue(Object obj) { + /** + * Converts a metadata object to a human-readable string for error messages. + * + *

Behavior: + *

    + *
  • If {@code obj} is {@code null}, returns {@code null}.
    • + *
  • Attempts to obtain a display name by invoking one of the getters + * {@code getDisplayText()}, {@code getDisplayName()}, or {@code getName()} via reflection. + * If a name is found, returns it quoted as {@code 'NAME'}.
    • + *
  • When the current calling account is a root admin, the returned value will include + * an identifier suffix in the form {@code (ID: id, UUID: uuid)} when available. + * The ID is included only if {@code INCLUDE_METADATA_ID_IN_MESSAGE} is {@code true} + * and {@code obj} implements {@link InternalIdentity}. The UUID is included when + * {@code obj} implements {@link org.apache.cloudstack.api.Identity}.
    • + *
  • If no display name is available, returns the UUID (if {@code obj} implements + * {@code Identity}); otherwise returns {@code obj.toString()}.
    • + *
+ * + *

Reflection is used to call getters; invocation failures are silently ignored and treated as + * absence of the corresponding value. + * + * @param obj metadata object + * @return formatted metadata string suitable for inclusion in error messages, or {@code null} + * if {@code obj} is {@code null} + */ + protected static String getMetadataObjectStringValue(Object obj) { if (obj == null) { return null; } - // obj is of primitive type - // String value structure should be 'NAME' (ID: id, UUID: uuid) - // NAME is obtained from obj.getName() or obj.getDisplayText() - // ID is obtained from obj.getId() if obj instanceof InternalIdentity and only for root admin - // UUID is obtained from obj.getUuid() if obj instanceof Identity - // If NAME is not available, fallback to obj.toString() then simply return UUID if available String uuid = null; if (obj instanceof Identity) { uuid = ((Identity) obj).getUuid(); @@ -156,7 +204,7 @@ public final class ErrorMessageResolver { return sb.toString(); } - private static String invokeStringGetter(Object obj, String methodName) { + protected static String invokeStringGetter(Object obj, String methodName) { try { Class cls = obj.getClass(); var m = cls.getMethod(methodName); @@ -167,28 +215,7 @@ public final class ErrorMessageResolver { } } - public static void updateExceptionResponse(ExceptionResponse response, CloudRuntimeException cre) { - String key = cre.getMessageKey(); - Map map = cre.getMetadata(); - - if (key == null) { - if (cre.getCause() instanceof InvalidParameterValueException) { - key = ((InvalidParameterValueException) cre.getCause()).getMessageKey(); - map = ((InvalidParameterValueException) cre.getCause()).getMetadata(); - } else { - return; - } - } - response.setErrorTextKey(key); - Map stringMap = getStringMap(map); - String message = getMessageUsingStringMap(key, stringMap); - if (message != null) { - response.setErrorText(message); - } - response.setErrorMetadata(stringMap); - } - - private static synchronized void reloadIfRequired() { + protected static synchronized void reloadIfRequired() { try { // log current directory for debugging purposes LOG.debug("Current working directory: {}", @@ -230,12 +257,8 @@ public final class ErrorMessageResolver { } } - private static String expand(String template, Map metadata) { - Map allMetadata = CallContext.current().getContextStringKeyParameters(); - if (MapUtils.isNotEmpty(allMetadata)) { - allMetadata.putAll(metadata); - } - if (MapUtils.isEmpty(allMetadata)) { + protected static String expand(String template, Map metadata) { + if (MapUtils.isEmpty(metadata)) { return template; } String result = template; @@ -248,4 +271,39 @@ public final class ErrorMessageResolver { } return result; } + + public static String getMessage(String errorKey, Map metadata) { + String template = getTemplateForKey(errorKey); + if (template == null) { + return errorKey; + } + Map combinedMetadata = getCombinedMetadataFromErrorTemplate(template, metadata); + return expand(template, getStringMap(combinedMetadata)); + } + + public static void updateExceptionResponse(ExceptionResponse response, CloudRuntimeException cre) { + String key = cre.getMessageKey(); + Map map = cre.getMetadata(); + + if (key == null) { + Throwable cause = cre.getCause(); + if (!(cause instanceof InvalidParameterValueException)) { + return; + } + InvalidParameterValueException ipve = (InvalidParameterValueException) cause; + key = ipve.getMessageKey(); + map = ipve.getMetadata(); + } + response.setErrorTextKey(key); + String template = getTemplateForKey(key); + if (template == null) { + response.setErrorText(key); + response.setErrorMetadata(getStringMap(map)); + return; + } + Map combinedMetadata = getCombinedMetadataFromErrorTemplate(template, map); + Map stringMap = getStringMap(combinedMetadata); + response.setErrorText(expand(template, stringMap)); + response.setErrorMetadata(stringMap); + } } diff --git a/api/src/test/java/org/apache/cloudstack/context/ErrorMessageResolverTest.java b/api/src/test/java/org/apache/cloudstack/context/ErrorMessageResolverTest.java new file mode 100644 index 00000000000..0c2f5575d25 --- /dev/null +++ b/api/src/test/java/org/apache/cloudstack/context/ErrorMessageResolverTest.java @@ -0,0 +1,460 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.context; + +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import org.apache.cloudstack.api.Identity; +import org.apache.cloudstack.api.response.ExceptionResponse; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.dc.DataCenter; +import com.cloud.exception.InvalidParameterValueException; +import com.cloud.utils.PropertiesUtil; +import com.cloud.utils.exception.CloudRuntimeException; + +@RunWith(MockitoJUnitRunner.class) +public class ErrorMessageResolverTest { + + @Mock + CallContext callContextMock; + + MockedStatic callContextMocked; + MockedStatic propertiesUtilMocked; + + Path tmpFile; + + @Before + public void setup() { + callContextMocked = Mockito.mockStatic(CallContext.class); + callContextMocked.when(CallContext::current).thenReturn(callContextMock); + propertiesUtilMocked = Mockito.mockStatic(PropertiesUtil.class); + propertiesUtilMocked.when(() -> PropertiesUtil.findConfigFile(anyString())).thenReturn(null); + } + + @After + public void tearDown() throws Exception { + callContextMocked.close(); + propertiesUtilMocked.close(); + if (tmpFile != null) { + try { + Files.deleteIfExists(tmpFile); + } catch (Exception ignored) { + } + } + } + + @Test + public void getVariableNamesInErrorKey_shouldReturnEmptyListForTemplateWithoutVariables() { + String template = "This is a template without variables."; + List result = ErrorMessageResolver.getVariableNamesInErrorKey(template); + Assert.assertTrue("Result should be an empty list", result.isEmpty()); + } + + @Test + public void getVariableNamesInErrorKey_shouldExtractSingleVariable() { + String template = "This is a template with one variable: {{variable}}."; + List result = ErrorMessageResolver.getVariableNamesInErrorKey(template); + Assert.assertEquals("Result should contain one variable", 1, result.size()); + Assert.assertEquals("Variable name should be 'variable'", "variable", result.get(0)); + } + + @Test + public void getVariableNamesInErrorKey_shouldExtractMultipleVariables() { + String template = "This template has {{var1}} and {{var2}}."; + List result = ErrorMessageResolver.getVariableNamesInErrorKey(template); + Assert.assertEquals("Result should contain two variables", 2, result.size()); + Assert.assertEquals("First variable name should be 'var1'", "var1", result.get(0)); + Assert.assertEquals("Second variable name should be 'var2'", "var2", result.get(1)); + } + + @Test + public void getVariableNamesInErrorKey_shouldIgnoreMalformedVariables() { + String template = "This template has {{var1} and {{var2}}."; + List result = ErrorMessageResolver.getVariableNamesInErrorKey(template); + Assert.assertEquals("Result should contain one valid variable", 1, result.size()); + Assert.assertEquals("Variable name should be 'var2'", "var2", result.get(0)); + } + + @Test + public void getVariableNamesInErrorKey_shouldHandleEmptyTemplate() { + String template = ""; + List result = ErrorMessageResolver.getVariableNamesInErrorKey(template); + Assert.assertTrue("Result should be an empty list", result.isEmpty()); + } + + @Test + public void getVariableNamesInErrorKey_shouldHandleTemplateWithOnlyVariables() { + String template = "{{var1}}{{var2}}{{var3}}"; + List result = ErrorMessageResolver.getVariableNamesInErrorKey(template); + Assert.assertEquals("Result should contain three variables", 3, result.size()); + Assert.assertEquals("First variable name should be 'var1'", "var1", result.get(0)); + Assert.assertEquals("Second variable name should be 'var2'", "var2", result.get(1)); + Assert.assertEquals("Third variable name should be 'var3'", "var3", result.get(2)); + } + + @Test + public void getCombinedMetadataFromErrorTemplate_shouldReturnMetadataWhenNoVariablesInTemplate() { + String template = "This is a template without variables."; + Map metadata = Map.of("key1", "value1"); + Map result = ErrorMessageResolver.getCombinedMetadataFromErrorTemplate(template, metadata); + Assert.assertEquals("Result should match the input metadata", metadata, result); + } + + @Test + public void getCombinedMetadataFromErrorTemplate_shouldReturnEmptyMapWhenContextMetadataIsEmpty() { + String template = "This template has {{var1}}."; + Map metadata = Map.of(); + when(callContextMock.getContextStringKeyParameters()).thenReturn(Map.of()); + Map result = ErrorMessageResolver.getCombinedMetadataFromErrorTemplate(template, metadata); + Assert.assertTrue("Result should be an empty map", result.isEmpty()); + } + + @Test + public void getCombinedMetadataFromErrorTemplate_shouldCombineContextAndProvidedMetadata() { + String template = "This template has {{var1}} and {{var2}}."; + Map metadata = Map.of("key1", "value1"); + when(callContextMock.getContextStringKeyParameters()).thenReturn(Map.of("var1", "valueVar1", "var2", "valueVar2")); + Map result = ErrorMessageResolver.getCombinedMetadataFromErrorTemplate(template, metadata); + Assert.assertEquals("Result should contain combined metadata", 3, result.size()); + Assert.assertEquals("Result should contain context metadata for var1", "valueVar1", result.get("var1")); + Assert.assertEquals("Result should contain context metadata for var2", "valueVar2", result.get("var2")); + Assert.assertEquals("Result should contain provided metadata", "value1", result.get("key1")); + } + + @Test + public void getCombinedMetadataFromErrorTemplate_shouldIgnoreVariablesNotInContextMetadata() { + String template = "This template has {{var1}} and {{var2}}."; + Map metadata = Map.of("key1", "value1"); + when(callContextMock.getContextStringKeyParameters()).thenReturn(Map.of("var1", "valueVar1")); + Map result = ErrorMessageResolver.getCombinedMetadataFromErrorTemplate(template, metadata); + Assert.assertEquals("Result should contain combined metadata", 2, result.size()); + Assert.assertEquals("Result should contain context metadata for var1", "valueVar1", result.get("var1")); + Assert.assertEquals("Result should contain provided metadata", "value1", result.get("key1")); + } + + @Test + public void getCombinedMetadataFromErrorTemplate_shouldReturnProvidedMetadataWhenTemplateIsEmpty() { + String template = ""; + Map metadata = Map.of("key1", "value1"); + Map result = ErrorMessageResolver.getCombinedMetadataFromErrorTemplate(template, metadata); + Assert.assertEquals("Result should match the input metadata", metadata, result); + } + + @Test + public void getStringMap_shouldReturnEmptyMapWhenMetadataIsEmpty() { + Map metadata = Map.of(); + Map result = ErrorMessageResolver.getStringMap(metadata); + Assert.assertTrue("Result should be an empty map", result.isEmpty()); + } + + @Test + public void getStringMap_shouldConvertAllMetadataValuesToStrings() { + Map metadata = Map.of("key1", 123, "key2", true, "key3", "value"); + Map result = ErrorMessageResolver.getStringMap(metadata); + Assert.assertEquals("Result should contain all keys", 3, result.size()); + Assert.assertEquals("Value for key1 should be '123'", "123", result.get("key1")); + Assert.assertEquals("Value for key2 should be 'true'", "true", result.get("key2")); + Assert.assertEquals("Value for key3 should be 'value'", "value", result.get("key3")); + } + + @Test + public void getStringMap_shouldHandleNullValuesInMetadata() { + Map metadata = new LinkedHashMap<>(); + metadata.put("key1", null); + metadata.put("key2", "value"); + Map result = ErrorMessageResolver.getStringMap(metadata); + Assert.assertEquals("Result should contain all keys", 2, result.size()); + Assert.assertNull("Value for key1 should be null", result.get("key1")); + Assert.assertEquals("Value for key2 should be 'value'", "value", result.get("key2")); + } + + @Test + public void getStringMap_shouldReturnEmptyMapWhenMetadataIsNull() { + Map result = ErrorMessageResolver.getStringMap(null); + Assert.assertTrue("Result should be an empty map", result.isEmpty()); + } + + @Test + public void getMetadataObjectStringValue_shouldReturnNullWhenObjectIsNull() { + Assert.assertNull(ErrorMessageResolver.getMetadataObjectStringValue(null)); + } + + @Test + public void getMetadataObjectStringValue_shouldReturnUuidWhenNameIsUnavailable() { + Identity identityMock = Mockito.mock(Identity.class); + when(identityMock.getUuid()).thenReturn("uuid-1234"); + Assert.assertEquals("uuid-1234", ErrorMessageResolver.getMetadataObjectStringValue(identityMock)); + } + + @Test + public void getMetadataObjectStringValue_shouldReturnNameWhenAvailable() { + DataCenter identityMock = Mockito.mock(DataCenter.class); + when(identityMock.getUuid()).thenReturn("uuid-1234"); + when(identityMock.getName()).thenReturn("TestName"); + Assert.assertEquals("'TestName'", ErrorMessageResolver.getMetadataObjectStringValue(identityMock)); + } + + @Test + public void getMetadataObjectStringValue_shouldIncludeIdAndUuidForRootAdmin() { + DataCenter internalIdentityMock = Mockito.mock(DataCenter.class); + when(internalIdentityMock.getUuid()).thenReturn("uuid-5678"); + if (ErrorMessageResolver.INCLUDE_METADATA_ID_IN_MESSAGE) { + when(internalIdentityMock.getId()).thenReturn(42L); + } + when(internalIdentityMock.getName()).thenReturn("AdminName"); + when(CallContext.current().isCallingAccountRootAdmin()).thenReturn(true); + String expected = String.format("'AdminName' (%sUUID: uuid-5678)", ErrorMessageResolver.INCLUDE_METADATA_ID_IN_MESSAGE ? "ID: 42, " : ""); + Assert.assertEquals(expected, ErrorMessageResolver.getMetadataObjectStringValue(internalIdentityMock)); + } + + @Test + public void getMetadataObjectStringValue_shouldFallbackToToStringWhenNameAndUuidAreUnavailable() { + Object obj = new Object(); + Assert.assertEquals(obj.toString(), ErrorMessageResolver.getMetadataObjectStringValue(obj)); + } + + @Test + public void getMetadataObjectStringValue_shouldReturnNameOnlyForNonRootAdmin() { + DataCenter internalIdentityMock = Mockito.mock(DataCenter.class); + when(internalIdentityMock.getName()).thenReturn("UserName"); + when(CallContext.current().isCallingAccountRootAdmin()).thenReturn(false); + Assert.assertEquals("'UserName'", ErrorMessageResolver.getMetadataObjectStringValue(internalIdentityMock)); + } + + @Test + public void expand_shouldReturnTemplateWhenMetadataIsEmpty() { + String template = "This is a template with no placeholders."; + Map metadata = Map.of(); + String result = ErrorMessageResolver.expand(template, metadata); + Assert.assertEquals("This is a template with no placeholders.", result); + } + + @Test + public void expand_shouldReplaceSinglePlaceholderWithMetadataValue() { + String template = "Hello, {{name}}!"; + Map metadata = Map.of("name", "World"); + String result = ErrorMessageResolver.expand(template, metadata); + Assert.assertEquals("Hello, World!", result); + } + + @Test + public void expand_shouldReplaceMultiplePlaceholdersWithMetadataValues() { + String template = "Hello, {{name}}! Today is {{day}}."; + Map metadata = Map.of("name", "Alice", "day", "Monday"); + String result = ErrorMessageResolver.expand(template, metadata); + Assert.assertEquals("Hello, Alice! Today is Monday.", result); + } + + @Test + public void expand_shouldIgnorePlaceholdersWithoutMatchingMetadata() { + String template = "Hello, {{name}}! Your age is {{age}}."; + Map metadata = Map.of("name", "Bob"); + String result = ErrorMessageResolver.expand(template, metadata); + Assert.assertEquals("Hello, Bob! Your age is {{age}}.", result); + } + + @Test + public void expand_shouldHandleNullMetadataValues() { + String template = "Hello, {{name}}!"; + Map metadata = new LinkedHashMap<>(); + metadata.put("name", null); + String result = ErrorMessageResolver.expand(template, metadata); + Assert.assertEquals("Hello, {{name}}!", result); + } + + @Test + public void expand_shouldReturnTemplateWhenMetadataIsNull() { + String template = "This is a template with no placeholders."; + String result = ErrorMessageResolver.expand(template, null); + Assert.assertEquals("This is a template with no placeholders.", result); + } + + @Test + public void expand_shouldHandleTemplateWithNoPlaceholders() { + String template = "This template has no placeholders."; + Map metadata = Map.of("key", "value"); + String result = ErrorMessageResolver.expand(template, metadata); + Assert.assertEquals("This template has no placeholders.", result); + } + + @Test + public void getMessage_shouldReturnErrorKeyWhenTemplateIsNull() { + String errorKey = "error.key"; + String result = ErrorMessageResolver.getMessage(errorKey, Map.of()); + Assert.assertEquals(errorKey, result); + } + + private void createTempFileWithTemplate(String errorKey, String template) { + try { + tmpFile = Files.createTempFile("error-template-", ".txt"); + Files.writeString(tmpFile, String.format("{ \"%s\": \"%s\" }", errorKey, template)); + propertiesUtilMocked.when(() -> PropertiesUtil.findConfigFile(anyString())) + .thenReturn(tmpFile.toFile()); + } catch (IOException e) { + Assert.fail("Failed to create temporary file for testing: " + e.getMessage()); + } + } + + @Test + public void getMessage_shouldReturnExpandedMessageWhenTemplateExists() { + String errorKey = "error.key"; + String template = "Error occurred: {{field}}"; + Map metadata = Map.of("field", "value"); + createTempFileWithTemplate(errorKey, template); + when(callContextMock.getContextStringKeyParameters()).thenReturn(Map.of()); + String result = ErrorMessageResolver.getMessage(errorKey, metadata); + Assert.assertEquals("Error occurred: value", result); + } + + @Test + public void getMessage_shouldHandleEmptyMetadata() { + String errorKey = "error.key"; + String template = "Error occurred."; + createTempFileWithTemplate(errorKey, template); + String result = ErrorMessageResolver.getMessage(errorKey, Map.of()); + Assert.assertEquals("Error occurred.", result); + } + + @Test + public void getMessage_shouldHandleNullMetadata() { + String errorKey = "error.key"; + String template = "Error occurred."; + createTempFileWithTemplate(errorKey, template); + String result = ErrorMessageResolver.getMessage(errorKey, null); + Assert.assertEquals("Error occurred.", result); + } + + @Test + public void getMessage_shouldCombineContextAndProvidedMetadata() { + String errorKey = "error.key"; + String template = "Error in {{field1}} and {{field2}}."; + Map metadata = Map.of("field1", "value1"); + createTempFileWithTemplate(errorKey, template); + when(callContextMock.getContextStringKeyParameters()).thenReturn(Map.of("field2", "value2")); + String result = ErrorMessageResolver.getMessage(errorKey, metadata); + Assert.assertEquals("Error in value1 and value2.", result); + } + + @Test + public void updateExceptionResponse_shouldSetErrorTextAndMetadataWhenKeyAndTemplateExist() { + ExceptionResponse response = new ExceptionResponse(); + CloudRuntimeException cre = Mockito.mock(CloudRuntimeException.class); + String key = "error.key"; + String template = "Error occurred: {{field}}"; + Map metadata = Map.of("field", "value"); + + when(cre.getMessageKey()).thenReturn(key); + when(cre.getMetadata()).thenReturn(metadata); + createTempFileWithTemplate(key, template); + + ErrorMessageResolver.updateExceptionResponse(response, cre); + + Assert.assertEquals(key, response.getErrorTextKey()); + Assert.assertEquals("Error occurred: value", response.getErrorText()); + Assert.assertEquals(Map.of("field", "value"), response.getErrorMetadata()); + } + + @Test + public void updateExceptionResponse_shouldSetErrorTextKeyOnlyWhenTemplateDoesNotExist() { + ExceptionResponse response = new ExceptionResponse(); + CloudRuntimeException cre = Mockito.mock(CloudRuntimeException.class); + String key = "error.key"; + Map metadata = Map.of("field", "value"); + + when(cre.getMessageKey()).thenReturn(key); + when(cre.getMetadata()).thenReturn(metadata); + + ErrorMessageResolver.updateExceptionResponse(response, cre); + + Assert.assertEquals(key, response.getErrorTextKey()); + Assert.assertEquals(key, response.getErrorText()); + Assert.assertEquals(Map.of("field", "value"), response.getErrorMetadata()); + } + + @Test + public void updateExceptionResponse_shouldHandleNullKeyAndCauseIsNotInvalidParameterValueException() { + ExceptionResponse response = new ExceptionResponse(); + String originalErrorText = response.getErrorText(); + CloudRuntimeException cre = Mockito.mock(CloudRuntimeException.class); + + when(cre.getMessageKey()).thenReturn(null); + when(cre.getCause()).thenReturn(new RuntimeException()); + + ErrorMessageResolver.updateExceptionResponse(response, cre); + + Assert.assertNull(response.getErrorTextKey()); + Assert.assertNull(response.getErrorMetadata()); + Assert.assertEquals(originalErrorText, response.getErrorText()); + } + + @Test + public void updateExceptionResponse_shouldUseCauseMetadataWhenKeyIsNullAndCauseIsInvalidParameterValueException() { + ExceptionResponse response = new ExceptionResponse(); + InvalidParameterValueException cause = Mockito.mock(InvalidParameterValueException.class); + CloudRuntimeException cre = Mockito.mock(CloudRuntimeException.class); + String key = "error.key"; + String template = "Error occurred: {{field}}"; + Map metadata = Map.of("field", "value"); + + when(cre.getMessageKey()).thenReturn(null); + when(cre.getCause()).thenReturn(cause); + when(cause.getMessageKey()).thenReturn(key); + when(cause.getMetadata()).thenReturn(metadata); + createTempFileWithTemplate(key, template); + + ErrorMessageResolver.updateExceptionResponse(response, cre); + + Assert.assertEquals(key, response.getErrorTextKey()); + Assert.assertEquals("Error occurred: value", response.getErrorText()); + Assert.assertEquals(Map.of("field", "value"), response.getErrorMetadata()); + } + + @Test + public void updateExceptionResponse_shouldHandleNullMetadata() { + ExceptionResponse response = new ExceptionResponse(); + CloudRuntimeException cre = Mockito.mock(CloudRuntimeException.class); + String key = "error.key"; + String template = "Error occurred."; + + when(cre.getMessageKey()).thenReturn(key); + when(cre.getMetadata()).thenReturn(null); + createTempFileWithTemplate(key, template); + + ErrorMessageResolver.updateExceptionResponse(response, cre); + + Assert.assertEquals(key, response.getErrorTextKey()); + Assert.assertEquals("Error occurred.", response.getErrorText()); + Assert.assertTrue(response.getErrorMetadata().isEmpty()); + } +} \ No newline at end of file diff --git a/client/conf/error-messages.json.in b/client/conf/error-messages.json.in index be840b3f87e..0dbfbc9008a 100644 --- a/client/conf/error-messages.json.in +++ b/client/conf/error-messages.json.in @@ -1,9 +1,21 @@ { - "vm.deploy.network.not.found.ip.map": "The network selected {{networkId}} in IP to network map could not be found. It may have been removed or is no longer accessible.", - "vm.deploy.serviceoffering.fixed.parameters.not.allowed": "Unable to deploy the instance because the selected service offering {{serviceOffering}} does not allow specifying {{cpuNumberKey}}, {{cpuSpeedKey}}, or {{memory}}.", - "vm.deploy.serviceoffering.fixed.parameters.not.allowed.admin": "Unable to deploy the instance because the selected service offering {{serviceOffering}} is not a dynamic offering and it does not allow specifying {{cpuNumberKey}}, {{cpuSpeedKey}}, or {{memory}}.", - "vm.deploy.serviceoffering.inactive": "Unable to deploy Instance as the given service offering {{serviceOffering}} is inactive. Specify an active service offering.", - "vm.deploy.serviceoffering.not.specified": "Unable to deploy Instance as the required parameter 'serviceofferingid' is missing.", - "vm.deploy.serviceoffering.override.not.allowed": "Unable to deploy Instance as the selected service offering {{serviceOffering}} does not allow changing the disk offering.", - "vm.deploy.serviceoffering.override.not.allowed.admin": "Unable to deploy Instance as the selected service offering {{serviceOffering}} uses disk offering strictness and does not allow changing the disk offering." + "vm.deploy.diskoffering.compute.only": "Unable to deploy Instance as the specified disk offering {{diskOffering}} is mapped to a service offering. Specify a disk offering that is not mapped to any service offering.", + "vm.deploy.diskoffering.local.storage.zone.unsupported": "Unable to deploy Instance because the zone is not configured to use local storage, but the specified disk offering {{diskOffering}} requires it.", + "vm.deploy.diskoffering.not.found": "Unable to deploy Instance as the specified disk offering is not found.", + "vm.deploy.diskoffering.with.diskoffering.details": "Unable to deploy Instance as both a disk offering ID and data disk offering details were provided. Specify only one.", + "vm.deploy.hypervisor.volume.snapshot.not.supported": "Unable to deploy Instance because deployment from an existing volume or snapshot is supported only on the KVM hypervisor.", + "vm.deploy.network.not.found.ip.map": "The network selected {{networkId}} in IP to network map could not be found. It may have been removed or is no longer accessible.", + "vm.deploy.serviceoffering.fixed.parameters.not.allowed": "Unable to deploy the instance because the selected service offering {{serviceOffering}} does not allow specifying {{cpuNumberKey}}, {{cpuSpeedKey}}, or {{memory}}.", + "vm.deploy.serviceoffering.fixed.parameters.not.allowed.admin": "Unable to deploy the instance because the selected service offering {{serviceOffering}} is not a dynamic offering and it does not allow specifying {{cpuNumberKey}}, {{cpuSpeedKey}}, or {{memory}}.", + "vm.deploy.serviceoffering.inactive": "Unable to deploy Instance as the given service offering {{serviceOffering}} is inactive. Specify an active service offering.", + "vm.deploy.serviceoffering.local.storage.zone.unsupported": "Unable to deploy Instance because the zone is not configured to use local storage, but the disk offering mapped to service offering {{serviceOffering}} requires it.", + "vm.deploy.serviceoffering.not.specified": "Unable to deploy Instance as the required parameter 'serviceofferingid' is missing.", + "vm.deploy.serviceoffering.not.found": "Unable to deploy Instance as the specified service offering is not found.", + "vm.deploy.serviceoffering.override.not.allowed": "Unable to deploy Instance as the selected service offering {{serviceOffering}} does not allow changing the disk offering.", + "vm.deploy.serviceoffering.override.not.allowed.admin": "Unable to deploy Instance as the selected service offering {{serviceOffering}} uses disk offering strictness and does not allow changing the disk offering.", + "vm.deploy.snapshot.not.found": "Unable to deploy Instance as the specified Snapshot is not found.", + "vm.deploy.template.associated.not.usable": "Unable to deploy Instance as the associated Template cannot be used.", + "vm.deploy.template.not.found": "Unable to deploy Instance as the specified Template is not found.", + "vm.deploy.volume.not.found": "Unable to deploy Instance as the specified Volume is not found.", + "vm.deploy.zone.not.found": "Unable to deploy Instance as the specified zone is not found." } diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 0d6e6cc32f7..b46d9703ccd 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -6383,7 +6383,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir DataCenter zone = _entityMgr.findById(DataCenter.class, zoneId); if (zone == null) { - throw new InvalidParameterValueException("Unable to find zone by id=" + zoneId); + throw new InvalidParameterValueException("vm.deploy.zone.not.found", Collections.emptyMap()); } Long serviceOfferingId = cmd.getServiceOfferingId(); @@ -6394,7 +6394,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir ServiceOffering serviceOffering = _entityMgr.findById(ServiceOffering.class, serviceOfferingId); if (serviceOffering == null) { - throw new InvalidParameterValueException("Unable to find service offering: " + serviceOffering.getId()); + throw new InvalidParameterValueException("vm.deploy.serviceoffering.not.found", Collections.emptyMap()); } verifyServiceOffering(cmd, serviceOffering); @@ -6408,14 +6408,14 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir if (cmd.getVolumeId() != null) { volume = getVolume(cmd.getVolumeId(), templateId, false); if (volume == null) { - throw new InvalidParameterValueException("Could not find volume with id=" + cmd.getVolumeId()); + throw new InvalidParameterValueException("vm.deploy.volume.not.found", Collections.emptyMap()); } _accountMgr.checkAccess(caller, null, true, volume); templateId = volume.getTemplateId(); } else if (cmd.getSnapshotId() != null) { snapshot = _snapshotDao.findById(cmd.getSnapshotId()); if (snapshot == null) { - throw new InvalidParameterValueException("Could not find snapshot with id=" + cmd.getSnapshotId()); + throw new InvalidParameterValueException("vm.deploy.snapshot.not.found", Collections.emptyMap()); } _accountMgr.checkAccess(caller, null, true, snapshot); VolumeInfo volumeOfSnapshot = getVolume(snapshot.getVolumeId(), templateId, true); @@ -6431,16 +6431,18 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir VirtualMachineTemplate template = null; if (volume != null || snapshot != null) { template = _entityMgr.findByIdIncludingRemoved(VirtualMachineTemplate.class, templateId); + if (template == null) { + throw new InvalidParameterValueException("vm.deploy.template.associated.not.usable", Collections.emptyMap()); + } } else { template = _entityMgr.findById(VirtualMachineTemplate.class, templateId); + if (template == null) { + throw new InvalidParameterValueException("vm.deploy.template.not.found", Collections.emptyMap()); + } } if (cmd.isVolumeOrSnapshotProvided() && (!(HypervisorType.KVM.equals(template.getHypervisorType()) || HypervisorType.KVM.equals(cmd.getHypervisor())))) { - throw new InvalidParameterValueException("Deploying a virtual machine with existing volume/snapshot is supported only from KVM hypervisors"); - } - // Make sure a valid template ID was specified - if (template == null) { - throw new InvalidParameterValueException("Unable to use template " + templateId); + throw new InvalidParameterValueException("vm.deploy.hypervisor.volume.snapshot.not.supported", Collections.emptyMap()); } verifyTemplate(cmd, template, serviceOfferingId); @@ -6449,25 +6451,29 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir if (diskOfferingId != null) { diskOffering = _entityMgr.findById(DiskOffering.class, diskOfferingId); if (diskOffering == null) { - throw new InvalidParameterValueException("Unable to find disk offering " + diskOfferingId); + throw new InvalidParameterValueException("vm.deploy.diskoffering.not.found", Collections.emptyMap()); } if (diskOffering.isComputeOnly()) { - throw new InvalidParameterValueException(String.format("The disk offering %s provided is directly mapped to a service offering, please provide an individual disk offering", diskOffering)); + throw new InvalidParameterValueException("vm.deploy.diskoffering.compute.only", + Map.of("diskOffering", diskOffering)); } } List dataDiskInfoList = cmd.getDataDiskInfoList(); if (dataDiskInfoList != null && diskOfferingId != null) { - new InvalidParameterValueException("Cannot specify both disk offering id and data disk offering details"); + throw new InvalidParameterValueException("vm.deploy.diskoffering.with.diskoffering.details", + Collections.emptyMap()); } if (!zone.isLocalStorageEnabled()) { DiskOffering diskOfferingMappedInServiceOffering = _entityMgr.findById(DiskOffering.class, serviceOffering.getDiskOfferingId()); if (diskOfferingMappedInServiceOffering.isUseLocalStorage()) { - throw new InvalidParameterValueException("Zone is not configured to use local storage but disk offering " + diskOfferingMappedInServiceOffering.getName() + " mapped in service offering uses it"); + throw new InvalidParameterValueException("vm.deploy.serviceoffering.local.storage.zone.unsupported", + Map.of("serviceOffering", serviceOffering)); } if (diskOffering != null && diskOffering.isUseLocalStorage()) { - throw new InvalidParameterValueException("Zone is not configured to use local storage but disk offering " + diskOffering.getName() + " uses it"); + throw new InvalidParameterValueException("vm.deploy.diskoffering.local.storage.zone.unsupported", + Map.of("diskOffering", diskOffering)); } }