Copilot commented on code in PR #13558:
URL: https://github.com/apache/gravitino/pull/13558#discussion_r4216912301
##########
core/src/test/java/org/apache/gravitino/storage/relational/service/TestJobTemplateMetaService.java:
##########
@@ -284,11 +294,86 @@ public void testUpdateJobTemplate() throws IOException {
.withAuditInfo(updatedJobTemplateEntity.auditInfo())
.build();
+ long lastChangeId = maxEntityChangeId();
Assertions.assertThrows(
EntityAlreadyExistsException.class,
() ->
jobTemplateMetaService.updateJobTemplate(
updatedJobTemplateEntity.nameIdentifier(), e ->
duplicateNameJobTemplateEntity));
+ // The rejected rename rolls back with its change-log record.
+ Assertions.assertTrue(jobTemplateChangesAfter(lastChangeId).isEmpty());
+ }
+
+ @TestTemplate
+ public void testRenameAndDeleteJobTemplateLogChanges() throws IOException {
Review Comment:
`@TestTemplate` requires a `TestTemplateInvocationContextProvider`
(typically via an extension). If none is registered for this class, JUnit will
error or skip execution, and this test won’t run as intended. If this is meant
to be a normal unit test, replace `@TestTemplate` with `@Test`; otherwise, add
the corresponding provider/extension for the template test.
##########
core/src/main/java/org/apache/gravitino/hook/TagHookDispatcher.java:
##########
@@ -90,12 +92,23 @@ public Tag createTag(
@Override
public Tag alterTag(String metalake, String name, TagChange... changes)
throws IllegalArgumentException, TagAlreadyExistsException {
- return dispatcher.alterTag(metalake, name, changes);
+ Tag alteredTag = dispatcher.alterTag(metalake, name, changes);
+ if (Arrays.stream(changes).anyMatch(change -> change instanceof
TagChange.RenameTag)) {
+ AuthorizationUtils.notifyEntityNameIdMappingChange(
+ NameIdentifierUtil.ofTag(metalake, name), Entity.EntityType.TAG);
+ }
+ return alteredTag;
}
Review Comment:
The rename-detection pattern (`Arrays.stream(...).anyMatch(change instanceof
...Rename...)`) is duplicated across multiple hook dispatchers in this PR.
Consider extracting a small shared helper (e.g., in `AuthorizationUtils` or a
package-private utility) to detect rename changes for each entity type,
reducing duplication and lowering the chance of inconsistent behavior if rename
change classes evolve.
--
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]
