Copilot commented on code in PR #9778:
URL: https://github.com/apache/gravitino/pull/9778#discussion_r2713144643
##########
api/src/main/java/org/apache/gravitino/authorization/Privileges.java:
##########
@@ -57,6 +57,14 @@ public class Privileges {
MetadataObject.Type.SCHEMA,
MetadataObject.Type.FILESET);
+ private static final Set<MetadataObject.Type> VIEW_SUPPORTED_TYPES =
+ Sets.immutableEnumSet(
+ MetadataObject.Type.METALAKE,
+ MetadataObject.Type.CATALOG,
+ MetadataObject.Type.SCHEMA,
+ MetadataObject.Type.VIEW);
+
+
Review Comment:
Remove extra blank line at line 67. There should be only one blank line
between the field declaration and the Javadoc comment.
```suggestion
```
##########
core/src/main/java/org/apache/gravitino/utils/NamespaceUtil.java:
##########
@@ -272,6 +284,19 @@ public static void checkTable(Namespace namespace) {
namespace);
}
+ /**
+ * Check if the given view namespace is legal, throw an {@link
IllegalNamespaceException} if it's
+ * illegal.
+ *
+ * @param namespace The view namespace
+ */
+ public static void checkView(Namespace namespace) {
+ check(
+ namespace != null && namespace.length() == 3,
+ "View namespace must be non-null and have 3 levels, the input
namespace is %s",
+ namespace);
+ }
Review Comment:
The checkView method should have test coverage to verify proper validation
of view namespaces, including tests for null namespaces, incorrect namespace
lengths, and valid 3-level namespaces.
##########
core/src/main/java/org/apache/gravitino/utils/NameIdentifierUtil.java:
##########
@@ -577,6 +602,11 @@ public static MetadataObject toMetadataObject(
String tableParent = dot.join(ident.namespace().level(1),
ident.namespace().level(2));
return MetadataObjects.of(tableParent, ident.name(),
MetadataObject.Type.TABLE);
+ case VIEW:
+ checkView(ident);
+ String viewParent = dot.join(ident.namespace().level(1),
ident.namespace().level(2));
+ return MetadataObjects.of(viewParent, ident.name(),
MetadataObject.Type.VIEW);
Review Comment:
The VIEW case in toMetadataObject should have test coverage to verify proper
namespace validation and metadata object construction for view identifiers.
##########
api/src/main/java/org/apache/gravitino/authorization/Privileges.java:
##########
@@ -1237,4 +1258,66 @@ public boolean canBindTo(MetadataObject.Type type) {
return type == MetadataObject.Type.METALAKE || type ==
MetadataObject.Type.JOB_TEMPLATE;
}
}
+
+ /** The privilege to create a view. */
+ public static class CreateView extends GenericPrivilege<CreateView> {
+ private static final CreateView ALLOW_INSTANCE =
+ new CreateView(Condition.ALLOW, Name.CREATE_VIEW);
+ private static final CreateView DENY_INSTANCE =
+ new CreateView(Condition.DENY, Name.CREATE_VIEW);
+
+ private CreateView(Condition condition, Name name) {
+ super(condition, name);
+ }
+
+ /**
+ * @return The instance with allow condition of the privilege.
+ */
+ public static CreateView allow() {
+ return ALLOW_INSTANCE;
+ }
+
+ /**
+ * @return The instance with deny condition of the privilege.
+ */
+ public static CreateView deny() {
+ return DENY_INSTANCE;
+ }
+
+ @Override
+ public boolean canBindTo(MetadataObject.Type type) {
+ return SCHEMA_SUPPORTED_TYPES.contains(type);
+ }
+ }
+
+ /** The privilege to select data from a view. */
+ public static class SelectView extends GenericPrivilege<SelectView> {
+ private static final SelectView ALLOW_INSTANCE =
+ new SelectView(Condition.ALLOW, Name.SELECT_VIEW);
+ private static final SelectView DENY_INSTANCE =
+ new SelectView(Condition.DENY, Name.SELECT_VIEW);
+
+ private SelectView(Condition condition, Name name) {
+ super(condition, name);
+ }
+
+ /**
+ * @return The instance with allow condition of the privilege.
+ */
+ public static SelectView allow() {
+ return ALLOW_INSTANCE;
+ }
+
+ /**
+ * @return The instance with deny condition of the privilege.
+ */
+ public static SelectView deny() {
+ return DENY_INSTANCE;
+ }
+
+ @Override
+ public boolean canBindTo(MetadataObject.Type type) {
+ return VIEW_SUPPORTED_TYPES.contains(type);
+ }
Review Comment:
The SelectView.canBindTo method should have test coverage to verify it
correctly validates binding to VIEW, SCHEMA, CATALOG, and METALAKE types, and
rejects other types.
##########
api/src/main/java/org/apache/gravitino/authorization/Privileges.java:
##########
@@ -1237,4 +1258,66 @@ public boolean canBindTo(MetadataObject.Type type) {
return type == MetadataObject.Type.METALAKE || type ==
MetadataObject.Type.JOB_TEMPLATE;
}
}
+
+ /** The privilege to create a view. */
+ public static class CreateView extends GenericPrivilege<CreateView> {
+ private static final CreateView ALLOW_INSTANCE =
+ new CreateView(Condition.ALLOW, Name.CREATE_VIEW);
+ private static final CreateView DENY_INSTANCE =
+ new CreateView(Condition.DENY, Name.CREATE_VIEW);
+
+ private CreateView(Condition condition, Name name) {
+ super(condition, name);
+ }
+
+ /**
+ * @return The instance with allow condition of the privilege.
+ */
+ public static CreateView allow() {
+ return ALLOW_INSTANCE;
+ }
+
+ /**
+ * @return The instance with deny condition of the privilege.
+ */
+ public static CreateView deny() {
+ return DENY_INSTANCE;
+ }
+
+ @Override
+ public boolean canBindTo(MetadataObject.Type type) {
+ return SCHEMA_SUPPORTED_TYPES.contains(type);
+ }
Review Comment:
The CreateView.canBindTo method should have test coverage to verify it
correctly validates binding to SCHEMA, CATALOG, METALAKE, and FILESET types,
and rejects VIEW and other types.
##########
core/src/main/java/org/apache/gravitino/utils/NameIdentifierUtil.java:
##########
@@ -466,6 +480,17 @@ public static void checkTable(NameIdentifier ident) {
NamespaceUtil.checkTable(ident.namespace());
}
+ /**
+ * Check the given {@link NameIdentifier} is a view identifier. Throw an
{@link
+ * IllegalNameIdentifierException} if it's not.
+ *
+ * @param ident The view {@link NameIdentifier} to check.
+ */
+ public static void checkView(NameIdentifier ident) {
+ NameIdentifier.check(ident != null, "View identifier must not be null");
+ NamespaceUtil.checkView(ident.namespace());
+ }
Review Comment:
The checkView method should have test coverage to verify proper validation
of view identifiers, including null checks and namespace validation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
