diff --git a/catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/HadoopCatalogOperations.java b/catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/HadoopCatalogOperations.java index 0aceee05e5b..215c5e3dcc2 100644 --- a/catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/HadoopCatalogOperations.java +++ b/catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/HadoopCatalogOperations.java @@ -250,6 +250,10 @@ public Fileset createMultipleLocationFileset( if (StringUtils.isBlank(name)) { throw new IllegalArgumentException("Location name must not be blank"); } + if (StringUtils.isBlank(path)) { + throw new IllegalArgumentException( + "Storage location must not be blank for location name: " + name); + } }); // Check if the fileset already existed in cache first. If it does, it means the fileset is @@ -974,7 +978,19 @@ private void validateLocationHierarchy( })); } + private boolean existBlankValue(Map properties, String key) { + return properties.containsKey(key) && StringUtils.isBlank(properties.get(key)); + } + private Map getAndCheckCatalogStorageLocations(Map properties) { + if (existBlankValue(properties, HadoopCatalogPropertiesMetadata.LOCATION)) { + // If the properties contain the catalog property "location", it must not be blank. + throw new IllegalArgumentException( + "The value of the catalog property " + + HadoopCatalogPropertiesMetadata.LOCATION + + " must not be blank"); + } + ImmutableMap.Builder catalogStorageLocations = ImmutableMap.builder(); String unnamedLocation = (String) @@ -989,11 +1005,17 @@ private Map getAndCheckCatalogStorageLocations(Map properties.forEach( (k, v) -> { - if (k.startsWith(PROPERTY_MULTIPLE_LOCATIONS_PREFIX) && StringUtils.isNotBlank(v)) { + if (k.startsWith(PROPERTY_MULTIPLE_LOCATIONS_PREFIX)) { String locationName = k.substring(PROPERTY_MULTIPLE_LOCATIONS_PREFIX.length()); if (StringUtils.isBlank(locationName)) { throw new IllegalArgumentException("Location name must not be blank"); } + + if (StringUtils.isBlank(v)) { + throw new IllegalArgumentException( + "Location value must not be blank for " + "location name: " + locationName); + } + checkPlaceholderValue(v); catalogStorageLocations.put(locationName, new Path((ensureTrailingSlash(v)))); } @@ -1003,6 +1025,14 @@ private Map getAndCheckCatalogStorageLocations(Map private Map getAndCheckSchemaPaths( String schemaName, Map schemaProps) { + if (existBlankValue(schemaProps, HadoopSchemaPropertiesMetadata.LOCATION)) { + // If the properties contain the schema property "location", it must not be blank. + throw new IllegalArgumentException( + "The value of the schema property " + + HadoopSchemaPropertiesMetadata.LOCATION + + " must not be blank"); + } + Map schemaPaths = new HashMap<>(); catalogStorageLocations.forEach( (name, path) -> { diff --git a/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/TestHadoopCatalogOperations.java b/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/TestHadoopCatalogOperations.java index 25c523ea709..775d7880a1f 100644 --- a/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/TestHadoopCatalogOperations.java +++ b/catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/TestHadoopCatalogOperations.java @@ -50,6 +50,7 @@ import java.net.ConnectException; import java.nio.file.Paths; import java.util.Arrays; +import java.util.HashMap; import java.util.Map; import java.util.Set; import java.util.UUID; @@ -382,15 +383,15 @@ public void testCreateSchemaWithEmptyCatalogLocation() throws IOException { String name = "schema28"; String comment = "comment28"; String catalogPath = ""; - Schema schema = createSchema(name, comment, catalogPath, null); - Assertions.assertEquals(name, schema.name()); - Assertions.assertEquals(comment, schema.comment()); Throwable exception = Assertions.assertThrows( - SchemaAlreadyExistsException.class, - () -> createSchema(name, comment, catalogPath, null)); - Assertions.assertEquals("Schema m1.c1.schema28 already exists", exception.getMessage()); + IllegalArgumentException.class, () -> createSchema(name, comment, catalogPath, null)); + Assertions.assertEquals( + "The value of the catalog property " + + HadoopCatalogPropertiesMetadata.LOCATION + + " must not be blank", + exception.getMessage()); } @Test @@ -1493,6 +1494,51 @@ public void testCreateMultipleLocationsWithExceptions() throws IOException { null)); Assertions.assertEquals("Location name must not be blank", exception.getMessage()); + // empty location in catalog location + Map illegalLocations2 = + new HashMap() { + { + put(PROPERTY_MULTIPLE_LOCATIONS_PREFIX + "v1", null); + } + }; + exception = + Assertions.assertThrows( + IllegalArgumentException.class, + () -> + ops.initialize( + illegalLocations2, + randomCatalogInfo("m1", "c1"), + HADOOP_PROPERTIES_METADATA)); + Assertions.assertEquals( + "Location value must not be blank for location name: v1", exception.getMessage()); + + // empty location in schema location + exception = + Assertions.assertThrows( + IllegalArgumentException.class, + () -> + createMultiLocationSchema( + "s1", "comment", ImmutableMap.of(), ImmutableMap.of("location", ""))); + Assertions.assertEquals( + "The value of the schema property location must not be blank", exception.getMessage()); + + // empty fileset storage location + exception = + Assertions.assertThrows( + IllegalArgumentException.class, + () -> + createMultiLocationFileset( + "fileset_test", + "s1", + null, + Fileset.Type.MANAGED, + ImmutableMap.of(), + ImmutableMap.of("location1", ""), + null)); + Assertions.assertEquals( + "Storage location must not be blank for location name: location1", + exception.getMessage()); + // storage location is parent of schema location Schema multipLocationSchema = createMultiLocationSchema(