Skip to content

Commit

Permalink
#29281: throwing exceotions when AppConfig.isEnabled() is false to b…
Browse files Browse the repository at this point in the history
…reak 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.
  • Loading branch information
victoralfaro-dotcms committed Aug 14, 2024
1 parent 8d6d344 commit 69eed78
Show file tree
Hide file tree
Showing 10 changed files with 92 additions and 45 deletions.
20 changes: 20 additions & 0 deletions dotCMS/src/main/java/com/dotcms/ai/app/AIAppUtil.java
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -141,6 +143,24 @@ public boolean discoverBooleanSecret(final Map<String, Secret> 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<String, Secret> 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);
}
Expand Down
25 changes: 14 additions & 11 deletions dotCMS/src/main/java/com/dotcms/ai/app/AIModels.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -155,19 +156,21 @@ public List<String> 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<String> 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<OpenAIModels> response = Try
.of(() -> fetchOpenAIModels(appConfig))
.getOrElseThrow(() -> new DotRuntimeException("Error fetching OpenAI supported models"));

final List<String> supported = response
.getResponse()
.getData()
.stream()
.map(OpenAIModel::getId)
.map(String::toLowerCase)
.collect(Collectors.toList());
supportedModelsCache.put(SUPPORTED_MODELS_KEY, supported);

return supported;
Expand Down
13 changes: 9 additions & 4 deletions dotCMS/src/main/java/com/dotcms/ai/app/AppConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -42,10 +47,10 @@ public AppConfig(final String host, final Map<String, Secret> 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(
Expand Down
2 changes: 1 addition & 1 deletion dotCMS/src/main/java/com/dotcms/ai/app/AppKeys.java
Original file line number Diff line number Diff line change
Expand Up @@ -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" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion dotCMS/src/main/java/com/dotcms/ai/util/OpenAIRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
3 changes: 2 additions & 1 deletion dotCMS/src/main/java/com/dotcms/security/apps/AppsUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,8 @@ private static String guessEnvVar(final String key, final String paramName) {
private static String discoverEnvVarValue(final Supplier<String> 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);
}
Expand Down
32 changes: 16 additions & 16 deletions dotCMS/src/main/resources/apps/dotAI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down
13 changes: 13 additions & 0 deletions dotCMS/src/test/java/com/dotcms/ai/app/AIAppUtilTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -43,7 +45,6 @@ public class AIModelsTest {
@BeforeClass
public static void beforeClass() throws Exception {
IntegrationTestInitService.getInstance().init();
IPUtils.disabledIpPrivateSubnet(true);
wireMockServer = AiTest.prepareWireMock();
}

Expand All @@ -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
Expand Down Expand Up @@ -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);
}

Expand All @@ -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);
Expand All @@ -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<String> supported = aiModels.getOrPullSupportedModels();
assertNotNull(supported);
assertTrue(supported.isEmpty());
aiModels.getOrPullSupportedModels();
}

private void saveSecrets(final Host host,
Expand Down

0 comments on commit 69eed78

Please sign in to comment.