Skip to content

Commit

Permalink
remove parameter
Browse files Browse the repository at this point in the history
  • Loading branch information
@jerqi
jerqi committed Oct 10, 2024
1 parent eff9b4a commit ab3f901
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 59 deletions.
7 changes: 3 additions & 4 deletions core/src/main/java/org/apache/gravitino/tag/TagManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import org.apache.gravitino.Entity;
import org.apache.gravitino.EntityAlreadyExistsException;
import org.apache.gravitino.EntityStore;
import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.Namespace;
Expand Down Expand Up @@ -244,7 +243,7 @@ public Tag[] listTagsInfoForMetadataObject(String metalake, MetadataObject metad
NameIdentifier entityIdent = MetadataObjectUtil.toEntityIdent(metalake, metadataObject);
Entity.EntityType entityType = MetadataObjectUtil.toEntityType(metadataObject);

MetadataObjectUtil.checkMetadataObject(metalake, metadataObject, GravitinoEnv.getInstance());
MetadataObjectUtil.checkMetadataObject(metalake, metadataObject);

return TreeLockUtils.doWithTreeLock(
entityIdent,
Expand All @@ -270,7 +269,7 @@ public Tag getTagForMetadataObject(String metalake, MetadataObject metadataObjec
Entity.EntityType entityType = MetadataObjectUtil.toEntityType(metadataObject);
NameIdentifier tagIdent = ofTagIdent(metalake, name);

MetadataObjectUtil.checkMetadataObject(metalake, metadataObject, GravitinoEnv.getInstance());
MetadataObjectUtil.checkMetadataObject(metalake, metadataObject);

return TreeLockUtils.doWithTreeLock(
entityIdent,
Expand Down Expand Up @@ -306,7 +305,7 @@ public String[] associateTagsForMetadataObject(
NameIdentifier entityIdent = MetadataObjectUtil.toEntityIdent(metalake, metadataObject);
Entity.EntityType entityType = MetadataObjectUtil.toEntityType(metadataObject);

MetadataObjectUtil.checkMetadataObject(metalake, metadataObject, GravitinoEnv.getInstance());
MetadataObjectUtil.checkMetadataObject(metalake, metadataObject);

// Remove all the tags that are both set to add and remove
Set<String> tagsToAddSet = tagsToAdd == null ? Sets.newHashSet() : Sets.newHashSet(tagsToAdd);
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/org/apache/gravitino/utils/MetadataObjectUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,10 @@ public static NameIdentifier toEntityIdent(String metalakeName, MetadataObject m
*
* @param metalake The metalake name
* @param object The metadata object
* @param env The Gravitino environment
* @throws NoSuchMetadataObjectException if the metadata object type doesn't exist.
*/
public static void checkMetadataObject(String metalake, MetadataObject object, GravitinoEnv env) {
public static void checkMetadataObject(String metalake, MetadataObject object) {
GravitinoEnv env = GravitinoEnv.getInstance();
NameIdentifier identifier = toEntityIdent(metalake, object);

Supplier<NoSuchMetadataObjectException> exceptionToThrowSupplier =
Expand Down
4 changes: 2 additions & 2 deletions server/src/main/java/org/apache/gravitino/server/web/rest/OwnerOperations.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public Response getOwnerForObject(
return Utils.doAs(
httpRequest,
() -> {
MetadataObjectUtil.checkMetadataObject(metalake, object, GravitinoEnv.getInstance());
MetadataObjectUtil.checkMetadataObject(metalake, object);
NameIdentifier ident = MetadataObjectUtil.toEntityIdent(metalake, object);
Optional<Owner> owner =
TreeLockUtils.doWithTreeLock(
Expand Down Expand Up @@ -113,7 +113,7 @@ public Response setOwnerForObject(
return Utils.doAs(
httpRequest,
() -> {
MetadataObjectUtil.checkMetadataObject(metalake, object, GravitinoEnv.getInstance());
MetadataObjectUtil.checkMetadataObject(metalake, object);
NameIdentifier objectIdent = MetadataObjectUtil.toEntityIdent(metalake, object);
TreeLockUtils.doWithTreeLock(
objectIdent,
Expand Down
4 changes: 2 additions & 2 deletions server/src/main/java/org/apache/gravitino/server/web/rest/PermissionOperations.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ public Response grantPrivilegeToRole(
AuthorizationUtils.checkPrivilege(privilegeDTO, object, metalake);
}

MetadataObjectUtil.checkMetadataObject(metalake, object, GravitinoEnv.getInstance());
MetadataObjectUtil.checkMetadataObject(metalake, object);
return TreeLockUtils.doWithTreeLock(
AuthorizationUtils.ofRole(metalake, role),
LockType.WRITE,
Expand Down Expand Up @@ -263,7 +263,7 @@ public Response revokePrivilegeFromRole(
AuthorizationUtils.checkPrivilege(privilegeDTO, object, metalake);
}

MetadataObjectUtil.checkMetadataObject(metalake, object, GravitinoEnv.getInstance());
MetadataObjectUtil.checkMetadataObject(metalake, object);
return TreeLockUtils.doWithTreeLock(
AuthorizationUtils.ofRole(metalake, role),
LockType.WRITE,
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ public Response createRole(@PathParam("metalake") String metalake, RoleCreateReq
for (Privilege privilege : object.privileges()) {
AuthorizationUtils.checkPrivilege((PrivilegeDTO) privilege, object, metalake);
}
MetadataObjectUtil.checkMetadataObject(metalake, object, GravitinoEnv.getInstance());
MetadataObjectUtil.checkMetadataObject(metalake, object);
}

List<SecurableObject> securableObjects =
Expand Down
5 changes: 2 additions & 3 deletions server/src/test/java/org/apache/gravitino/server/web/rest/TestOwnerOperations.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,12 +257,11 @@ void testSetOwnerForObject() {
public void testRoleObject() {
MetadataObject role = MetadataObjects.of(null, "role", MetadataObject.Type.ROLE);
when(accessControlDispatcher.getRole(any(), any())).thenReturn(mock(Role.class));
Assertions.assertDoesNotThrow(
() -> MetadataObjectUtil.checkMetadataObject("metalake", role, GravitinoEnv.getInstance()));
Assertions.assertDoesNotThrow(() -> MetadataObjectUtil.checkMetadataObject("metalake", role));

doThrow(new NoSuchRoleException("test")).when(accessControlDispatcher).getRole(any(), any());
Assertions.assertThrows(
NoSuchMetadataObjectException.class,
() -> MetadataObjectUtil.checkMetadataObject("metalake", role, GravitinoEnv.getInstance()));
() -> MetadataObjectUtil.checkMetadataObject("metalake", role));
}
}
60 changes: 15 additions & 45 deletions server/src/test/java/org/apache/gravitino/server/web/rest/TestRoleOperations.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -441,79 +441,59 @@ public void testCheckSecurableObjects() {
SecurableObjects.ofCatalog("catalog", Lists.newArrayList(Privileges.UseCatalog.allow()));
when(catalogDispatcher.catalogExists(any())).thenReturn(true);
Assertions.assertDoesNotThrow(
() ->
MetadataObjectUtil.checkMetadataObject(
"metalake", DTOConverters.toDTO(catalog), GravitinoEnv.getInstance()));
() -> MetadataObjectUtil.checkMetadataObject("metalake", DTOConverters.toDTO(catalog)));
when(catalogDispatcher.catalogExists(any())).thenReturn(false);
Assertions.assertThrows(
NoSuchMetadataObjectException.class,
() ->
MetadataObjectUtil.checkMetadataObject(
"metalake", DTOConverters.toDTO(catalog), GravitinoEnv.getInstance()));
() -> MetadataObjectUtil.checkMetadataObject("metalake", DTOConverters.toDTO(catalog)));

// check the schema
SecurableObject schema =
SecurableObjects.ofSchema(
catalog, "schema", Lists.newArrayList(Privileges.UseSchema.allow()));
when(schemaDispatcher.schemaExists(any())).thenReturn(true);
Assertions.assertDoesNotThrow(
() ->
MetadataObjectUtil.checkMetadataObject(
"metalake", DTOConverters.toDTO(schema), GravitinoEnv.getInstance()));
() -> MetadataObjectUtil.checkMetadataObject("metalake", DTOConverters.toDTO(schema)));
when(schemaDispatcher.schemaExists(any())).thenReturn(false);
Assertions.assertThrows(
NoSuchMetadataObjectException.class,
() ->
MetadataObjectUtil.checkMetadataObject(
"metalake", DTOConverters.toDTO(schema), GravitinoEnv.getInstance()));
() -> MetadataObjectUtil.checkMetadataObject("metalake", DTOConverters.toDTO(schema)));

// check the table
SecurableObject table =
SecurableObjects.ofTable(
schema, "table", Lists.newArrayList(Privileges.SelectTable.allow()));
when(tableDispatcher.tableExists(any())).thenReturn(true);
Assertions.assertDoesNotThrow(
() ->
MetadataObjectUtil.checkMetadataObject(
"metalake", DTOConverters.toDTO(table), GravitinoEnv.getInstance()));
() -> MetadataObjectUtil.checkMetadataObject("metalake", DTOConverters.toDTO(table)));
when(tableDispatcher.tableExists(any())).thenReturn(false);
Assertions.assertThrows(
NoSuchMetadataObjectException.class,
() ->
MetadataObjectUtil.checkMetadataObject(
"metalake", DTOConverters.toDTO(table), GravitinoEnv.getInstance()));
() -> MetadataObjectUtil.checkMetadataObject("metalake", DTOConverters.toDTO(table)));

// check the topic
SecurableObject topic =
SecurableObjects.ofTopic(
schema, "topic", Lists.newArrayList(Privileges.ConsumeTopic.allow()));
when(topicDispatcher.topicExists(any())).thenReturn(true);
Assertions.assertDoesNotThrow(
() ->
MetadataObjectUtil.checkMetadataObject(
"metalake", DTOConverters.toDTO(topic), GravitinoEnv.getInstance()));
() -> MetadataObjectUtil.checkMetadataObject("metalake", DTOConverters.toDTO(topic)));
when(topicDispatcher.topicExists(any())).thenReturn(false);
Assertions.assertThrows(
NoSuchMetadataObjectException.class,
() ->
MetadataObjectUtil.checkMetadataObject(
"metalake", DTOConverters.toDTO(topic), GravitinoEnv.getInstance()));
() -> MetadataObjectUtil.checkMetadataObject("metalake", DTOConverters.toDTO(topic)));

// check the fileset
SecurableObject fileset =
SecurableObjects.ofFileset(
schema, "fileset", Lists.newArrayList(Privileges.ReadFileset.allow()));
when(filesetDispatcher.filesetExists(any())).thenReturn(true);
Assertions.assertDoesNotThrow(
() ->
MetadataObjectUtil.checkMetadataObject(
"metalake", DTOConverters.toDTO(fileset), GravitinoEnv.getInstance()));
() -> MetadataObjectUtil.checkMetadataObject("metalake", DTOConverters.toDTO(fileset)));
when(filesetDispatcher.filesetExists(any())).thenReturn(false);
Assertions.assertThrows(
NoSuchMetadataObjectException.class,
() ->
MetadataObjectUtil.checkMetadataObject(
"metalake", DTOConverters.toDTO(fileset), GravitinoEnv.getInstance()));
() -> MetadataObjectUtil.checkMetadataObject("metalake", DTOConverters.toDTO(fileset)));

AtomicReference<String> wrongParent = new AtomicReference<>();
AtomicReference<String> wrongName = new AtomicReference<>();
Expand Down Expand Up @@ -544,45 +524,35 @@ public Type type() {
wrongType.set(MetadataObject.Type.CATALOG);
Assertions.assertThrows(
IllegalNamespaceException.class,
() ->
MetadataObjectUtil.checkMetadataObject(
"metalake", wrongMetadataObject, GravitinoEnv.getInstance()));
() -> MetadataObjectUtil.checkMetadataObject("metalake", wrongMetadataObject));

// Test schema object
wrongType.set(MetadataObject.Type.CATALOG);
wrongParent.set("catalog1.schema1");
Assertions.assertThrows(
IllegalNamespaceException.class,
() ->
MetadataObjectUtil.checkMetadataObject(
"metalake", wrongMetadataObject, GravitinoEnv.getInstance()));
() -> MetadataObjectUtil.checkMetadataObject("metalake", wrongMetadataObject));

// Test table object
wrongType.set(MetadataObject.Type.TABLE);
wrongParent.set("catalog1");
Assertions.assertThrows(
IllegalNamespaceException.class,
() ->
MetadataObjectUtil.checkMetadataObject(
"metalake", wrongMetadataObject, GravitinoEnv.getInstance()));
() -> MetadataObjectUtil.checkMetadataObject("metalake", wrongMetadataObject));

// Test fileset object
wrongType.set(MetadataObject.Type.FILESET);
wrongParent.set("catalog1");
Assertions.assertThrows(
IllegalNamespaceException.class,
() ->
MetadataObjectUtil.checkMetadataObject(
"metalake", wrongMetadataObject, GravitinoEnv.getInstance()));
() -> MetadataObjectUtil.checkMetadataObject("metalake", wrongMetadataObject));

// Test topic object
wrongType.set(MetadataObject.Type.TOPIC);
wrongParent.set("catalog1");
Assertions.assertThrows(
IllegalNamespaceException.class,
() ->
MetadataObjectUtil.checkMetadataObject(
"metalake", wrongMetadataObject, GravitinoEnv.getInstance()));
() -> MetadataObjectUtil.checkMetadataObject("metalake", wrongMetadataObject));
}

@Test
Expand Down

0 comments on commit ab3f901

Please sign in to comment.