Make some fields in manifest file as mandatory (opensearch-project#289)

Signed-off-by: Heemin Kim <[email protected]>
heemin32 · Jul 21, 2023 · 82e3430 · 82e3430
1 parent 6d5ffa5
commit 82e3430
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 19 deletions.
diff --git a/src/main/java/org/opensearch/geospatial/ip2geo/action/PutDatasourceRequest.java b/src/main/java/org/opensearch/geospatial/ip2geo/action/PutDatasourceRequest.java
@@ -176,7 +176,7 @@ private void validateManifestFile(final URL url, final ActionRequestValidationEx
             manifest = DatasourceManifest.Builder.build(url);
         } catch (Exception e) {
             log.info("Error occurred while reading a file from {}", url, e);
-            errors.addValidationError(String.format(Locale.ROOT, "Error occurred while reading a file from %s", url));
+            errors.addValidationError(String.format(Locale.ROOT, "Error occurred while reading a file from %s: %s", url, e.getMessage()));
             return;
         }
 
@@ -188,7 +188,7 @@ private void validateManifestFile(final URL url, final ActionRequestValidationEx
             return;
         }
 
-        if (updateInterval.days() >= manifest.getValidForInDays()) {
+        if (manifest.getValidForInDays() != null && updateInterval.days() >= manifest.getValidForInDays()) {
             errors.addValidationError(
                 String.format(
                     Locale.ROOT,

diff --git a/src/main/java/org/opensearch/geospatial/ip2geo/common/DatasourceManifest.java b/src/main/java/org/opensearch/geospatial/ip2geo/common/DatasourceManifest.java
@@ -93,12 +93,12 @@ public class DatasourceManifest {
         }
     );
     static {
-        PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), URL_FIELD);
-        PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), DB_NAME_FIELD);
-        PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), SHA256_HASH_FIELD);
+        PARSER.declareString(ConstructingObjectParser.constructorArg(), URL_FIELD);
+        PARSER.declareString(ConstructingObjectParser.constructorArg(), DB_NAME_FIELD);
+        PARSER.declareString(ConstructingObjectParser.constructorArg(), SHA256_HASH_FIELD);
         PARSER.declareLong(ConstructingObjectParser.optionalConstructorArg(), VALID_FOR_IN_DAYS_FIELD);
-        PARSER.declareLong(ConstructingObjectParser.optionalConstructorArg(), UPDATED_AT_FIELD);
-        PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), PROVIDER_FIELD);
+        PARSER.declareLong(ConstructingObjectParser.constructorArg(), UPDATED_AT_FIELD);
+        PARSER.declareString(ConstructingObjectParser.constructorArg(), PROVIDER_FIELD);
     }
 
     /**

diff --git a/src/test/java/org/opensearch/geospatial/ip2geo/action/PutDatasourceRequestTests.java b/src/test/java/org/opensearch/geospatial/ip2geo/action/PutDatasourceRequestTests.java
@@ -23,7 +23,7 @@
 
 public class PutDatasourceRequestTests extends Ip2GeoTestCase {
 
-    public void testValidateWithInvalidUrl() {
+    public void testValidate_whenInvalidUrl_thenFails() {
         String datasourceName = GeospatialTestHelper.randomLowerCaseString();
         PutDatasourceRequest request = new PutDatasourceRequest(datasourceName);
         request.setEndpoint("invalidUrl");
@@ -33,29 +33,26 @@ public void testValidateWithInvalidUrl() {
         assertEquals("Invalid URL format is provided", exception.validationErrors().get(0));
     }
 
-    public void testValidateWithInvalidManifestFile() {
+    public void testValidate_whenInvalidManifestFile_thenFails() {
         String datasourceName = GeospatialTestHelper.randomLowerCaseString();
         String domain = GeospatialTestHelper.randomLowerCaseString();
         PutDatasourceRequest request = new PutDatasourceRequest(datasourceName);
         request.setEndpoint(String.format(Locale.ROOT, "https://%s.com", domain));
         request.setUpdateInterval(TimeValue.timeValueDays(1));
         ActionRequestValidationException exception = request.validate();
         assertEquals(1, exception.validationErrors().size());
-        assertEquals(
-            String.format(Locale.ROOT, "Error occurred while reading a file from %s", request.getEndpoint()),
-            exception.validationErrors().get(0)
-        );
+        assertTrue(exception.validationErrors().get(0).contains("Error occurred while reading a file"));
     }
 
-    public void testValidate() throws Exception {
+    public void testValidate_whenValidInput_thenSucceed() throws Exception {
         String datasourceName = GeospatialTestHelper.randomLowerCaseString();
         PutDatasourceRequest request = new PutDatasourceRequest(datasourceName);
         request.setEndpoint(sampleManifestUrl());
         request.setUpdateInterval(TimeValue.timeValueDays(1));
         assertNull(request.validate());
     }
 
-    public void testValidateWithZeroUpdateInterval() throws Exception {
+    public void testValidate_whenZeroUpdateInterval_thenFails() throws Exception {
         String datasourceName = GeospatialTestHelper.randomLowerCaseString();
         PutDatasourceRequest request = new PutDatasourceRequest(datasourceName);
         request.setEndpoint(sampleManifestUrl());
@@ -72,7 +69,7 @@ public void testValidateWithZeroUpdateInterval() throws Exception {
         );
     }
 
-    public void testValidateWithLargeUpdateInterval() throws Exception {
+    public void testValidate_whenLargeUpdateInterval_thenFail() throws Exception {
         String datasourceName = GeospatialTestHelper.randomLowerCaseString();
         PutDatasourceRequest request = new PutDatasourceRequest(datasourceName);
         request.setEndpoint(sampleManifestUrl());
@@ -86,7 +83,7 @@ public void testValidateWithLargeUpdateInterval() throws Exception {
         assertTrue(exception.validationErrors().get(0).contains("should be smaller"));
     }
 
-    public void testValidateWithInvalidUrlInsideManifest() throws Exception {
+    public void testValidate_whenInvalidUrlInsideManifest_thenFail() throws Exception {
         String datasourceName = GeospatialTestHelper.randomLowerCaseString();
         PutDatasourceRequest request = new PutDatasourceRequest(datasourceName);
         request.setEndpoint(sampleManifestUrlWithInvalidUrl());
@@ -100,7 +97,7 @@ public void testValidateWithInvalidUrlInsideManifest() throws Exception {
         assertTrue(exception.validationErrors().get(0).contains("Invalid URL format"));
     }
 
-    public void testValidateDatasourceNames() throws Exception {
+    public void testValidate_whenInvalidDatasourceNames_thenFails() throws Exception {
         String validDatasourceName = GeospatialTestHelper.randomLowerCaseString();
         String domain = GeospatialTestHelper.randomLowerCaseString();
         PutDatasourceRequest request = new PutDatasourceRequest(validDatasourceName);
@@ -154,7 +151,7 @@ public void testValidateDatasourceNames() throws Exception {
         }
     }
 
-    public void testStreamInOut() throws Exception {
+    public void testStreamInOut_whenValidInput_thenSucceed() throws Exception {
         String datasourceName = GeospatialTestHelper.randomLowerCaseString();
         String domain = GeospatialTestHelper.randomLowerCaseString();
         PutDatasourceRequest request = new PutDatasourceRequest(datasourceName);