Skip to content

Commit

Permalink
refactor: renamed files, efficient loading of resources, better excep…
Browse files Browse the repository at this point in the history
…tion handling

Signed-off-by: Pavan Yekbote <[email protected]>
  • Loading branch information
pyek-bot committed Dec 27, 2024
1 parent 399b892 commit 59998c2
Show file tree
Hide file tree
Showing 15 changed files with 67 additions and 51 deletions.
18 changes: 9 additions & 9 deletions common/src/main/java/org/opensearch/ml/common/CommonValue.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,15 @@ public class CommonValue {
public static final Set<String> stopWordsIndices = ImmutableSet.of(".plugins-ml-stop-words");

// Index mapping paths
public static final String ML_MODEL_GROUP_INDEX_MAPPING_PATH = "index-mappings/ml-model-group.json";
public static final String ML_MODEL_INDEX_MAPPING_PATH = "index-mappings/ml-model.json";
public static final String ML_TASK_INDEX_MAPPING_PATH = "index-mappings/ml-task.json";
public static final String ML_CONNECTOR_INDEX_MAPPING_PATH = "index-mappings/ml-connector.json";
public static final String ML_CONFIG_INDEX_MAPPING_PATH = "index-mappings/ml-config.json";
public static final String ML_CONTROLLER_INDEX_MAPPING_PATH = "index-mappings/ml-controller.json";
public static final String ML_AGENT_INDEX_MAPPING_PATH = "index-mappings/ml-agent.json";
public static final String ML_MEMORY_META_INDEX_MAPPING_PATH = "index-mappings/ml-memory-meta.json";
public static final String ML_MEMORY_MESSAGE_INDEX_MAPPING_PATH = "index-mappings/ml-memory-message.json";
public static final String ML_MODEL_GROUP_INDEX_MAPPING_PATH = "index-mappings/ml_model_group.json";
public static final String ML_MODEL_INDEX_MAPPING_PATH = "index-mappings/ml_model.json";
public static final String ML_TASK_INDEX_MAPPING_PATH = "index-mappings/ml_task.json";
public static final String ML_CONNECTOR_INDEX_MAPPING_PATH = "index-mappings/ml_connector.json";
public static final String ML_CONFIG_INDEX_MAPPING_PATH = "index-mappings/ml_config.json";
public static final String ML_CONTROLLER_INDEX_MAPPING_PATH = "index-mappings/ml_controller.json";
public static final String ML_AGENT_INDEX_MAPPING_PATH = "index-mappings/ml_agent.json";
public static final String ML_MEMORY_META_INDEX_MAPPING_PATH = "index-mappings/ml_memory_meta.json";
public static final String ML_MEMORY_MESSAGE_INDEX_MAPPING_PATH = "index-mappings/ml_memory_message.json";

// Calculate Versions independently of OpenSearch core version
public static final Version VERSION_2_11_0 = Version.fromString("2.11.0");
Expand Down
47 changes: 30 additions & 17 deletions common/src/main/java/org/opensearch/ml/common/utils/IndexUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import java.io.IOException;
import java.net.URL;
import java.util.HashMap;
import java.util.Map;

import com.google.common.base.Charsets;
Expand Down Expand Up @@ -73,34 +74,46 @@ public static String replacePlaceholders(String mapping) throws IOException {
throw new IllegalArgumentException("Mapping cannot be null or empty");
}

// Preload resources into memory to avoid redundant I/O
Map<String, String> loadedPlaceholders = new HashMap<>();
for (Map.Entry<String, String> placeholder : MAPPING_PLACEHOLDERS.entrySet()) {
URL url = IndexUtils.class.getClassLoader().getResource(placeholder.getValue());
if (url == null) {
throw new IOException("Resource not found: " + placeholder.getValue());
}

String placeholderMapping = Resources.toString(url, Charsets.UTF_8);
mapping = mapping.replace(placeholder.getKey(), placeholderMapping);
loadedPlaceholders.put(placeholder.getKey(), Resources.toString(url, Charsets.UTF_8));
}

return mapping;
StringBuilder result = new StringBuilder(mapping);
for (Map.Entry<String, String> entry : loadedPlaceholders.entrySet()) {
String placeholder = entry.getKey();
String replacement = entry.getValue();

// Replace all occurrences of the placeholder
int index;
while ((index = result.indexOf(placeholder)) != -1) {
result.replace(index, index + placeholder.length(), replacement);
}
}

return result.toString();
}

/**
- Checks if mapping is a valid json
- Validates mapping against a schema found in mappings/schema.json
- Schema validates the following:
- Below fields are present:
- "_meta"
- "_meta.schema_version"
- "properties"
- No additional fields at root level
- No additional fields in "_meta" object
- "properties" is an object type
- "_meta" is an object type
- "_meta_.schema_version" provided type is integer
Note: Validation can be made more strict if a specific schema is defined for each index.
* Checks if mapping is a valid json
* Validates mapping against a schema found in mappings/schema.json
* Schema validates the following:
* Below fields are present:
* "_meta"
* "_meta.schema_version"
* "properties"
* No additional fields at root level
* No additional fields in "_meta" object
* "properties" is an object type
* "_meta" is an object type
* "_meta_.schema_version" provided type is integer
* Note: Validation can be made more strict if a specific schema is defined for each index.
*/
public static void validateMapping(String mapping) throws IOException {
if (mapping.isBlank() || !StringUtils.isJson(mapping)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,28 @@

package org.opensearch.ml.common.utils;

import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.security.AccessController;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import org.apache.commons.lang3.BooleanUtils;
import org.json.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;
import org.opensearch.OpenSearchParseException;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.gson.Gson;
Expand Down Expand Up @@ -347,25 +347,28 @@ public static JsonObject getJsonObjectFromString(String jsonString) {
return JsonParser.parseString(jsonString).getAsJsonObject();
}

public static void validateSchema(String schemaString, String instanceString) throws IOException {
// parse the schema JSON as string
JsonNode schemaNode = MAPPER.readTree(schemaString);
JsonSchema schema = JsonSchemaFactory.getInstance(SpecVersion.VersionFlag.V202012).getSchema(schemaNode);

// JSON data to validate
JsonNode jsonNode = MAPPER.readTree(instanceString);

// Validate JSON node against the schema
Set<ValidationMessage> errors = schema.validate(jsonNode);
if (!errors.isEmpty()) {
throw new OpenSearchParseException(
"Validation failed: "
+ Arrays.toString(errors.toArray(new ValidationMessage[0]))
+ " for instance: "
+ instanceString
+ " with schema: "
+ schemaString
);
public static void validateSchema(String schemaString, String instanceString) {
try {
// parse the schema JSON as string
JsonNode schemaNode = MAPPER.readTree(schemaString);
JsonSchema schema = JsonSchemaFactory.getInstance(SpecVersion.VersionFlag.V202012).getSchema(schemaNode);

// JSON data to validate
JsonNode jsonNode = MAPPER.readTree(instanceString);

// Validate JSON node against the schema
Set<ValidationMessage> errors = schema.validate(jsonNode);
if (!errors.isEmpty()) {
String errorMessage = errors.stream().map(ValidationMessage::getMessage).collect(Collectors.joining(", "));

throw new OpenSearchParseException(
"Validation failed: " + errorMessage + " for instance: " + instanceString + " with schema: " + schemaString
);
}
} catch (JsonProcessingException e) {
throw new IllegalArgumentException("Invalid JSON format: " + e.getMessage(), e);
} catch (Exception e) {
throw new OpenSearchParseException("Schema validation failed: " + e.getMessage(), e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,23 +67,23 @@ public void testGetMappingFromFile() {
+ " }\n"
+ "}\n";
try {
String actualMapping = IndexUtils.getMappingFromFile("index-mappings/test-mapping.json");
String actualMapping = IndexUtils.getMappingFromFile("index-mappings/test_mapping.json");
// comparing JsonObjects to avoid issues caused by eol character in different OS
assertEquals(StringUtils.getJsonObjectFromString(expectedMapping), StringUtils.getJsonObjectFromString(actualMapping));
} catch (IOException e) {
throw new RuntimeException("Failed to read file at path: index-mappings/test-mapping.json");
throw new RuntimeException("Failed to read file at path: index-mappings/test_mapping.json");
}
}

@Test
public void testGetMappingFromFileFileNotFound() {
String path = "index-mappings/test-mapping-not-found.json";
String path = "index-mappings/test_mapping_not_found.json";
IOException e = assertThrows(IOException.class, () -> IndexUtils.getMappingFromFile(path));
}

@Test
public void testGetMappingFromFilesMalformedJson() {
String path = "index-mappings/test-mapping-malformed.json";
String path = "index-mappings/test_mapping_malformed.json";
IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> IndexUtils.getMappingFromFile(path));
}

Expand Down

0 comments on commit 59998c2

Please sign in to comment.