Improve the code

Author: jerryshao

api/src/main/java/org/apache/gravitino/Catalog.java

@@ -40,19 +40,42 @@ public interface Catalog extends Auditable {
   /** The type of the catalog. */
   enum Type {
     /** Catalog Type for Relational Data Structure, like db.table, catalog.db.table. */
-    RELATIONAL,
+    RELATIONAL(false),
 
     /** Catalog Type for Fileset System (including HDFS, S3, etc.), like path/to/file */
-    FILESET,
+    FILESET(false),
 
     /** Catalog Type for Message Queue, like Kafka://topic */
-    MESSAGING,
+    MESSAGING(false),
 
     /** Catalog Type for ML model */
-    MODEL,
+    MODEL(true),
 
     /** Catalog Type for test only. */
-    UNSUPPORTED;
+    UNSUPPORTED(false);
+
+    private final boolean supportsManagedCatalog;
+
+    Type(boolean supportsManagedCatalog) {
+      this.supportsManagedCatalog = supportsManagedCatalog;
+    }
+
+    /**
+     * A flag to indicate whether the catalog type supports managed catalog. Managed catalog is a
+     * concept in Gravitino, for the details of managed catalog, please refer to the class comment
+     * of {@link CatalogProvider}. If the catalog type supports managed catalog, users can create
+     * managed catalog of this type without specifying the provider, Gravitino will use the type as
+     * the provider to create the managed catalog. If the catalog type does not support managed
+     * catalog, users need to specify the provider when creating the catalog.
+     *
+     * <p>Currently, only the model catalog supports managed catalog.
+     *
+     * @return Whether the catalog type supports managed catalog. Returns true if the catalog type
+     *     supports managed catalog.
+     */
+    public boolean supportsManagedCatalog() {
+      return supportsManagedCatalog;
+    }
 
     /**
      * Convert the string (case-insensitive) to the catalog type.

api/src/main/java/org/apache/gravitino/CatalogProvider.java

@@ -24,18 +24,32 @@
 /**
  * A Catalog provider is a class that provides a short name for a catalog. This short name is used
  * when creating a catalog.
+ *
+ * <p>There are two kinds of catalogs in Gravitino, one is managed catalog and another is external
+ * catalog.
+ *
+ * <p>Managed catalog: A catalog and its subsidiary objects are all managed by Gravitino. Gravitino
+ * takes care of the lifecycle of the catalog and its objects. For those catalogs, Gravitino uses
+ * the type of the catalog as the provider short name. Note that for each catalog type, there is
+ * only one implementation of managed catalog for that type. Currently, Gravitino only has model
+ * catalog that is a managed catalog.
+ *
+ * <p>External catalog: A catalog its subsidiary objects are stored by an external sources, such as
+ * Hive catalog, the DB and tables are stored in HMS. For those catalogs, Gravitino uses a unique
+ * name as the provider short name to load the catalog. For example, Hive catalog uses "hive" as the
+ * provider short name.
  */
 @Evolving
 public interface CatalogProvider {
 
   /**
-   * Form the provider short name for a builtin catalog. The provider short name for a builtin
+   * Form the provider short name for a managed catalog. The provider short name for a managed
    * catalog is the catalog type in lowercase.
    *
    * @param type The catalog type.
-   * @return The provider short name for the builtin catalog.
+   * @return The provider short name for the managed catalog.
    */
-  static String builtinShortName(Catalog.Type type) {
+  static String shortNameForManagedCatalog(Catalog.Type type) {
     return type.name().toLowerCase(Locale.ROOT);
   }
 

api/src/main/java/org/apache/gravitino/SupportsCatalogs.java

@@ -81,11 +81,12 @@ default boolean catalogExists(String catalogName) {
    * catalog should be created. The short name:
    *
    * <p>1) should be the same as the {@link CatalogProvider} interface provided. 2) can be "null" if
-   * the created catalog is a built-in catalog, like model catalog.
+   * the created catalog is the managed catalog, like model catalog. For the details of the provider
+   * definition, see {@link CatalogProvider}.
    *
    * @param catalogName the name of the catalog.
    * @param type the type of the catalog.
-   * @param provider the provider of the catalog, or null if the catalog is a built-in catalog.
+   * @param provider the provider of the catalog, or null if the catalog is a managed catalog.
    * @param comment the comment of the catalog.
    * @param properties the properties of the catalog.
    * @return The created catalog.
@@ -101,7 +102,7 @@ Catalog createCatalog(
       throws NoSuchMetalakeException, CatalogAlreadyExistsException;
 
   /**
-   * Create a built-in catalog with specified catalog name, type, comment, and properties.
+   * Create a managed catalog with specified catalog name, type, comment, and properties.
    *
    * @param catalogName the name of the catalog.
    * @param type the type of the catalog.

catalogs/catalog-model/src/main/java/org/apache/gravitino/catalog/model/ModelCatalog.java

@@ -38,7 +38,7 @@ public class ModelCatalog extends BaseCatalog<ModelCatalog> {
 
   @Override
   public String shortName() {
-    return CatalogProvider.builtinShortName(super.type());
+    return CatalogProvider.shortNameForManagedCatalog(super.type());
   }
 
   @Override

common/src/main/java/org/apache/gravitino/dto/requests/CatalogCreateRequest.java

@@ -82,14 +82,25 @@ public CatalogCreateRequest(
   }
 
   /**
-   * Sets the provider of the catalog.
+   * Sets the provider of the catalog if it is null. The value of provider in the request can be
+   * null if the catalog is a managed catalog. For such request, the value will be set when it is
+   * deserialized.
    *
    * @param provider The provider of the catalog.
    */
   @JsonSetter(value = "provider")
   public void setProvider(String provider) {
-    this.provider =
-        StringUtils.isNotBlank(provider) ? provider : CatalogProvider.builtinShortName(type);
+    if (StringUtils.isNotBlank(provider)) {
+      this.provider = provider;
+    } else if (type != null && type.supportsManagedCatalog()) {
+      this.provider = CatalogProvider.shortNameForManagedCatalog(type);
+    } else {
+      throw new IllegalStateException(
+          "Provider cannot be null for catalog type "
+              + type
+              + " that doesn't support managed "
+              + "catalog");
+    }
   }
 
   /**
@@ -102,5 +113,10 @@ public void validate() throws IllegalArgumentException {
     Preconditions.checkArgument(
         StringUtils.isNotBlank(name), "\"name\" field is required and cannot be empty");
     Preconditions.checkArgument(type != null, "\"type\" field is required and cannot be empty");
+    Preconditions.checkArgument(
+        StringUtils.isNotBlank(provider) || type.supportsManagedCatalog(),
+        "\"provider\" field is required and cannot be empty for catalog type "
+            + type
+            + " that doesn't support managed catalog");
   }
 }

common/src/test/java/org/apache/gravitino/dto/requests/TestCatalogCreateRequest.java

@@ -19,6 +19,7 @@
 package org.apache.gravitino.dto.requests;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonMappingException;
 import com.google.common.collect.ImmutableMap;
 import java.util.Locale;
 import org.apache.gravitino.Catalog;
@@ -33,7 +34,7 @@ public void testCatalogCreateRequestSerDe() throws JsonProcessingException {
     CatalogCreateRequest request =
         new CatalogCreateRequest(
             "catalog_test",
-            Catalog.Type.RELATIONAL,
+            Catalog.Type.MODEL,
             "provider_test",
             "catalog comment",
             ImmutableMap.of("key", "value"));
@@ -44,14 +45,14 @@ public void testCatalogCreateRequestSerDe() throws JsonProcessingException {
 
     Assertions.assertEquals(request, deserRequest);
     Assertions.assertEquals("catalog_test", deserRequest.getName());
-    Assertions.assertEquals(Catalog.Type.RELATIONAL, deserRequest.getType());
+    Assertions.assertEquals(Catalog.Type.MODEL, deserRequest.getType());
     Assertions.assertEquals("provider_test", deserRequest.getProvider());
     Assertions.assertEquals("catalog comment", deserRequest.getComment());
     Assertions.assertEquals(ImmutableMap.of("key", "value"), deserRequest.getProperties());
 
     // Test with null provider, comment and properties
     CatalogCreateRequest request1 =
-        new CatalogCreateRequest("catalog_test", Catalog.Type.RELATIONAL, null, null, null);
+        new CatalogCreateRequest("catalog_test", Catalog.Type.MODEL, null, null, null);
 
     String serJson1 = JsonUtils.objectMapper().writeValueAsString(request1);
     CatalogCreateRequest deserRequest1 =
@@ -61,5 +62,14 @@ public void testCatalogCreateRequestSerDe() throws JsonProcessingException {
         deserRequest1.getType().name().toLowerCase(Locale.ROOT), deserRequest1.getProvider());
     Assertions.assertNull(deserRequest1.getComment());
     Assertions.assertNull(deserRequest1.getProperties());
+
+    // Test using null provider with catalog type doesn't support managed catalog
+    CatalogCreateRequest request2 =
+        new CatalogCreateRequest("catalog_test", Catalog.Type.RELATIONAL, null, null, null);
+
+    String serJson2 = JsonUtils.objectMapper().writeValueAsString(request2);
+    Assertions.assertThrows(
+        JsonMappingException.class,
+        () -> JsonUtils.objectMapper().readValue(serJson2, CatalogCreateRequest.class));
   }
 }