From 655c96a806473edd612673094071cc0c1a965bb8 Mon Sep 17 00:00:00 2001 From: Victor Alfaro Date: Tue, 13 Aug 2024 16:46:47 -0600 Subject: [PATCH] #29281: throwing exceotions when AppConfig.isEnabled() is false to break the tests when any of AI API url or AI API key are missing. Clean the default values for model names and max tokens are now required. --- .../java/com/dotcms/ai/app/AIAppUtil.java | 20 ++++++++++++ .../main/java/com/dotcms/ai/app/AIModels.java | 25 ++++++++------- .../java/com/dotcms/ai/app/AppConfig.java | 13 +++++--- .../main/java/com/dotcms/ai/app/AppKeys.java | 2 +- .../ai/listener/EmbeddingContentListener.java | 5 ++- .../com/dotcms/ai/util/OpenAIRequest.java | 3 +- .../com/dotcms/security/apps/AppsUtil.java | 3 +- dotCMS/src/main/resources/apps/dotAI.yml | 32 +++++++++---------- .../java/com/dotcms/ai/app/AIAppUtilTest.java | 13 ++++++++ .../java/com/dotcms/ai/app/AIModelsTest.java | 21 ++++++------ 10 files changed, 92 insertions(+), 45 deletions(-) diff --git a/dotCMS/src/main/java/com/dotcms/ai/app/AIAppUtil.java b/dotCMS/src/main/java/com/dotcms/ai/app/AIAppUtil.java index 63f58056609a..c6667e844c26 100644 --- a/dotCMS/src/main/java/com/dotcms/ai/app/AIAppUtil.java +++ b/dotCMS/src/main/java/com/dotcms/ai/app/AIAppUtil.java @@ -1,6 +1,8 @@ package com.dotcms.ai.app; +import com.dotcms.security.apps.AppsUtil; import com.dotcms.security.apps.Secret; +import com.dotmarketing.util.UtilMethods; import com.liferay.util.StringPool; import io.vavr.Lazy; import io.vavr.control.Try; @@ -141,6 +143,24 @@ public boolean discoverBooleanSecret(final Map secrets, final Ap return Boolean.parseBoolean(discoverSecret(secrets, key)); } + /** + * Resolves a secret value from the provided secrets map using the specified key and environment variable. + * If the secret is not found in the secrets map, it attempts to discover the value from the environment variable. + * + * @param secrets the map of secrets + * @param key the key to look up the secret + * @param envVar the environment variable name to look up if the secret is not found in the secrets map + * @return the resolved secret value or the value from the environment variable if the secret is not found + */ + public String discoverEnvSecret(final Map secrets, final AppKeys key, final String envVar) { + final String secret = discoverSecret(secrets, key); + if (UtilMethods.isSet(secret)) { + return secret; + } + + return AppsUtil.discoverEnvVarValue(AppKeys.APP_KEY, key.key, envVar); + } + private int toInt(final String value) { return Try.of(() -> Integer.parseInt(value)).getOrElse(0); } diff --git a/dotCMS/src/main/java/com/dotcms/ai/app/AIModels.java b/dotCMS/src/main/java/com/dotcms/ai/app/AIModels.java index b47f2d12a392..26e397f03237 100644 --- a/dotCMS/src/main/java/com/dotcms/ai/app/AIModels.java +++ b/dotCMS/src/main/java/com/dotcms/ai/app/AIModels.java @@ -4,6 +4,7 @@ import com.dotcms.ai.model.OpenAIModels; import com.dotcms.ai.model.SimpleModel; import com.dotcms.http.CircuitBreakerUrl; +import com.dotmarketing.exception.DotRuntimeException; import com.dotmarketing.util.Config; import com.dotmarketing.util.Logger; import com.github.benmanes.caffeine.cache.Cache; @@ -155,19 +156,21 @@ public List getOrPullSupportedModels() { final AppConfig appConfig = appConfigSupplier.get(); if (!appConfig.isEnabled()) { - Logger.debug(this, "OpenAI is not enabled, returning empty list of supported models"); - return List.of(); + AppConfig.debugLogger(getClass(), () -> "dotAI is not enabled, returning empty list of supported models"); + throw new DotRuntimeException("App dotAI config without API urls or API key"); } - final List supported = Try.of(() -> - fetchOpenAIModels(appConfig) - .getResponse() - .getData() - .stream() - .map(OpenAIModel::getId) - .map(String::toLowerCase) - .collect(Collectors.toList())) - .getOrElse(Optional.ofNullable(cached).orElse(List.of())); + final CircuitBreakerUrl.Response response = Try + .of(() -> fetchOpenAIModels(appConfig)) + .getOrElseThrow(() -> new DotRuntimeException("Error fetching OpenAI supported models")); + + final List supported = response + .getResponse() + .getData() + .stream() + .map(OpenAIModel::getId) + .map(String::toLowerCase) + .collect(Collectors.toList()); supportedModelsCache.put(SUPPORTED_MODELS_KEY, supported); return supported; diff --git a/dotCMS/src/main/java/com/dotcms/ai/app/AppConfig.java b/dotCMS/src/main/java/com/dotcms/ai/app/AppConfig.java index ca315356009e..fe1c7d067b05 100644 --- a/dotCMS/src/main/java/com/dotcms/ai/app/AppConfig.java +++ b/dotCMS/src/main/java/com/dotcms/ai/app/AppConfig.java @@ -21,6 +21,11 @@ */ public class AppConfig implements Serializable { + private static final String AI_API_KEY_KEY = "AI_API_KEY"; + private static final String AI_API_URL_KEY = "AI_API_URL"; + private static final String AI_IMAGE_API_URL_KEY = "AI_IMAGE_API_URL"; + private static final String AI_EMBEDDINGS_API_URL_KEY = "AI_EMBEDDINGS_API_URL"; + public static final Pattern SPLITTER = Pattern.compile("\\s?,\\s?"); private final String host; @@ -42,10 +47,10 @@ public AppConfig(final String host, final Map secrets) { this.host = host; final AIAppUtil aiAppUtil = AIAppUtil.get(); - apiKey = aiAppUtil.discoverSecret(secrets, AppKeys.API_KEY); - apiUrl = aiAppUtil.discoverSecret(secrets, AppKeys.API_URL); - apiImageUrl = aiAppUtil.discoverSecret(secrets, AppKeys.API_IMAGE_URL); - apiEmbeddingsUrl = aiAppUtil.discoverSecret(secrets, AppKeys.API_EMBEDDINGS_URL); + apiKey = aiAppUtil.discoverEnvSecret(secrets, AppKeys.API_KEY, AI_API_KEY_KEY); + apiUrl = aiAppUtil.discoverEnvSecret(secrets, AppKeys.API_URL, AI_API_URL_KEY); + apiImageUrl = aiAppUtil.discoverEnvSecret(secrets, AppKeys.API_IMAGE_URL, AI_IMAGE_API_URL_KEY); + apiEmbeddingsUrl = aiAppUtil.discoverEnvSecret(secrets, AppKeys.API_EMBEDDINGS_URL, AI_EMBEDDINGS_API_URL_KEY); if (!secrets.isEmpty() || isEnabled()) { AIModels.get().loadModels( diff --git a/dotCMS/src/main/java/com/dotcms/ai/app/AppKeys.java b/dotCMS/src/main/java/com/dotcms/ai/app/AppKeys.java index d6a955294897..7afdad1c380b 100644 --- a/dotCMS/src/main/java/com/dotcms/ai/app/AppKeys.java +++ b/dotCMS/src/main/java/com/dotcms/ai/app/AppKeys.java @@ -2,10 +2,10 @@ public enum AppKeys { + API_KEY("apiKey", null), API_URL("apiUrl", "https://api.openai.com/v1/chat/completions"), API_IMAGE_URL("apiImageUrl", "https://api.openai.com/v1/images/generations"), API_EMBEDDINGS_URL("apiEmbeddingsUrl", "https://api.openai.com/v1/embeddings"), - API_KEY("apiKey", null), ROLE_PROMPT( "rolePrompt", "You are dotCMSbot, and AI assistant to help content" + diff --git a/dotCMS/src/main/java/com/dotcms/ai/listener/EmbeddingContentListener.java b/dotCMS/src/main/java/com/dotcms/ai/listener/EmbeddingContentListener.java index 16d4a5c26f0f..9739bab313eb 100644 --- a/dotCMS/src/main/java/com/dotcms/ai/listener/EmbeddingContentListener.java +++ b/dotCMS/src/main/java/com/dotcms/ai/listener/EmbeddingContentListener.java @@ -83,7 +83,10 @@ private AppConfig getAppConfig(final String hostId) { final AppConfig appConfig = ConfigService.INSTANCE.config(host); if (!appConfig.isEnabled()) { - throw new DotRuntimeException("No API urls or API key found in app config"); + AppConfig.debugLogger( + getClass(), + () -> "dotAI is not enabled since no API urls or API key found in app config"); + throw new DotRuntimeException("App dotAI config without API urls or API key"); } return appConfig; diff --git a/dotCMS/src/main/java/com/dotcms/ai/util/OpenAIRequest.java b/dotCMS/src/main/java/com/dotcms/ai/util/OpenAIRequest.java index 4e916e976db7..d022156f15e3 100644 --- a/dotCMS/src/main/java/com/dotcms/ai/util/OpenAIRequest.java +++ b/dotCMS/src/main/java/com/dotcms/ai/util/OpenAIRequest.java @@ -54,7 +54,8 @@ public static void doRequest(final String urlIn, final OutputStream out) { if (!appConfig.isEnabled()) { - throw new DotRuntimeException("OpenAI is not enabled and will not send request."); + AppConfig.debugLogger(OpenAIRequest.class, () -> "dotAI is not enabled and will not send request."); + throw new DotRuntimeException("App dotAI config without API urls or API key"); } final AIModel model = appConfig.resolveModelOrThrow(json.optString(AiKeys.MODEL)); diff --git a/dotCMS/src/main/java/com/dotcms/security/apps/AppsUtil.java b/dotCMS/src/main/java/com/dotcms/security/apps/AppsUtil.java index 3a7b32847215..271d51e1c59b 100644 --- a/dotCMS/src/main/java/com/dotcms/security/apps/AppsUtil.java +++ b/dotCMS/src/main/java/com/dotcms/security/apps/AppsUtil.java @@ -676,7 +676,8 @@ private static String guessEnvVar(final String key, final String paramName) { private static String discoverEnvVarValue(final Supplier envVarSupplier, final String envVar) { return Optional .ofNullable(envVarSupplier.get()) - .map(discovered -> Config.getStringProperty(discovered, null)) + .map(supplied -> Config.getStringProperty(supplied, null)) + .or(() -> Optional.ofNullable(envVar).map(ev -> Config.getStringProperty(ev, null))) .or(() -> Optional.ofNullable(envVar).map(System::getenv)) .orElse(null); } diff --git a/dotCMS/src/main/resources/apps/dotAI.yml b/dotCMS/src/main/resources/apps/dotAI.yml index 6ad3a46c6ed6..d23962e1e4f0 100644 --- a/dotCMS/src/main/resources/apps/dotAI.yml +++ b/dotCMS/src/main/resources/apps/dotAI.yml @@ -14,6 +14,13 @@ params: label: "API Key" hint: "Your ChatGPT API key" required: true + textModelNames: + value: "" + hidden: false + type: "STRING" + label: "Model Names" + hint: "Comma delimited list of models used to generate OpenAI API response (e.g. gpt-3.5-turbo-16k)" + required: true rolePrompt: value: "You are dotCMSbot, and AI assistant to help content creators generate and rewrite content in their content management system." hidden: false @@ -28,13 +35,6 @@ params: label: "Text Prompt" hint: "A prompt describing writing style." required: false - textModelNames: - value: "gpt-3.5-turbo-16k" - hidden: false - type: "STRING" - label: "Model Names" - hint: "Comma delimited list of models used to generate OpenAI API response." - required: true textModelTokensPerMinute: value: "180000" hidden: false @@ -63,6 +63,13 @@ params: label: "Completion model enabled" hint: "Enable completion model used to generate OpenAI API response." required: false + imageModelNames: + value: "" + hidden: false + type: "STRING" + label: "Image Model Names" + hint: "Comma delimited list of image models used to generate OpenAI API response(e.g. dall-e-3)." + required: true imagePrompt: value: "Use 16:9 aspect ratio." hidden: false @@ -96,13 +103,6 @@ params: value: "1920x1080" - label: "256x256 (Small Square 1:1)" value: "256x256" - imageModelNames: - value: "dall-e-3" - hidden: false - type: "STRING" - label: "Image Model Names" - hint: "Comma delimited list of image models used to generate OpenAI API response." - required: true imageModelTokensPerMinute: value: "0" hidden: false @@ -132,11 +132,11 @@ params: hint: "Enable completion model used to generate OpenAI API response." required: false embeddingsModelNames: - value: "text-embedding-ada-002" + value: "" hidden: false type: "STRING" label: "Embeddings Model Names" - hint: "Comma delimited list of embeddings models used to generate OpenAI API response." + hint: "Comma delimited list of embeddings models used to generate OpenAI API response (e.g. text-embedding-ada-002)." required: true embeddingsModelTokensPerMinute: value: "1000000" diff --git a/dotCMS/src/test/java/com/dotcms/ai/app/AIAppUtilTest.java b/dotCMS/src/test/java/com/dotcms/ai/app/AIAppUtilTest.java index a95eca8a4d5c..c4d5c93b7627 100644 --- a/dotCMS/src/test/java/com/dotcms/ai/app/AIAppUtilTest.java +++ b/dotCMS/src/test/java/com/dotcms/ai/app/AIAppUtilTest.java @@ -162,4 +162,17 @@ public void testCreateEmbeddingsModel() { assertTrue(model.getNames().contains("embeddingsmodel")); } + @Test + public void testDiscoverEnvSecret() { + // Mock the secret value in the secrets map + when(secrets.get("apiKey")).thenReturn(secret); + when(secret.getString()).thenReturn("secretValue"); + + // Call the method with the key and environment variable + String result = aiAppUtil.discoverEnvSecret(secrets, AppKeys.API_KEY, "ENV_API_KEY"); + + // Assert the expected outcome + assertEquals("secretValue", result); + } + } \ No newline at end of file diff --git a/dotcms-integration/src/test/java/com/dotcms/ai/app/AIModelsTest.java b/dotcms-integration/src/test/java/com/dotcms/ai/app/AIModelsTest.java index 2ea51fe91ab4..15483a02ad4e 100644 --- a/dotcms-integration/src/test/java/com/dotcms/ai/app/AIModelsTest.java +++ b/dotcms-integration/src/test/java/com/dotcms/ai/app/AIModelsTest.java @@ -7,10 +7,12 @@ import com.dotmarketing.beans.Host; import com.dotmarketing.business.APILocator; import com.dotmarketing.exception.DotDataException; +import com.dotmarketing.exception.DotRuntimeException; import com.dotmarketing.exception.DotSecurityException; import com.dotmarketing.util.DateUtil; import com.github.tomakehurst.wiremock.WireMockServer; import io.vavr.control.Try; +import org.junit.After; import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; @@ -43,7 +45,6 @@ public class AIModelsTest { @BeforeClass public static void beforeClass() throws Exception { IntegrationTestInitService.getInstance().init(); - IPUtils.disabledIpPrivateSubnet(true); wireMockServer = AiTest.prepareWireMock(); } @@ -55,11 +56,17 @@ public static void afterClass() { @Before public void before() { + IPUtils.disabledIpPrivateSubnet(true); host = new SiteDataGen().nextPersisted(); otherHost = new SiteDataGen().nextPersisted(); List.of(host, otherHost).forEach(h -> Try.of(() -> AiTest.aiAppSecrets(wireMockServer, host)).get()); } + @After + public void after() { + IPUtils.disabledIpPrivateSubnet(false); + } + /** * Given a set of models loaded into the AIModels instance * When the findModel method is called with various model names and types @@ -142,10 +149,6 @@ public void test_getOrPullSupportedModules() throws DotDataException, DotSecurit assertNotNull(supported); assertEquals(32, supported.size()); - supported = aiModels.getOrPullSupportedModels(); - assertNotNull(supported); - assertEquals(32, supported.size()); - AIModels.get().setAppConfigSupplier(ConfigService.INSTANCE::config); } @@ -154,7 +157,7 @@ public void test_getOrPullSupportedModules() throws DotDataException, DotSecurit * When the getOrPullSupportedModules method is called * Then an empty list of supported models should be returned. */ - @Test + @Test(expected = DotRuntimeException.class) public void test_getOrPullSupportedModules_invalidEndpoint() { AIModels.get().cleanSupportedModelsCache(); IPUtils.disabledIpPrivateSubnet(false); @@ -172,14 +175,12 @@ public void test_getOrPullSupportedModules_invalidEndpoint() { * When the getOrPullSupportedModules method is called * Then an empty list of supported models should be returned. */ - @Test + @Test(expected = DotRuntimeException.class) public void test_getOrPullSupportedModules_noApiKey() throws DotDataException, DotSecurityException { AiTest.aiAppSecrets(wireMockServer, APILocator.systemHost(), null); AIModels.get().cleanSupportedModelsCache(); - final List supported = aiModels.getOrPullSupportedModels(); - assertNotNull(supported); - assertTrue(supported.isEmpty()); + aiModels.getOrPullSupportedModels(); } private void saveSecrets(final Host host,