LuciferYang commented on code in PR #13440:
URL: https://github.com/apache/gravitino/pull/13440#discussion_r4114146863
##########
core/src/main/java/org/apache/gravitino/secret/SecretAlterChanges.java:
##########
@@ -79,19 +89,54 @@ public static Pair<CatalogChange[], List<SecretMaterial>>
prepareCatalogChanges(
out.add(CatalogChange.setProperty(c.getProperty(), value));
} else if (change instanceof CatalogChange.RemoveProperty) {
CatalogChange.RemoveProperty c = (CatalogChange.RemoveProperty)
change;
- secretManager.alterRemoveProperty(properties, "catalog", entityId,
c.getProperty());
+ secretManager.alterRemoveProperty(
+ properties, "catalog", entityId, c.getProperty(), replacedUrns);
out.add(change);
} else {
out.add(change);
}
}
- return Pair.of(out.toArray(new CatalogChange[0]), List.copyOf(written));
+ return Pair.of(
+ out.toArray(new CatalogChange[0]),
+ holderOf(written, replacedUrns, properties, originalProperties));
} catch (RuntimeException e) {
- secretManager.rollbackSecrets(written);
+ // Roll back only materials the persisted entity cannot reference. An
+ // in-batch re-bind may have rewritten a deterministic URN that the
original
+ // properties still point at, so deleting it would dangle the URN.
+ secretManager.rollbackSecrets(rollbackSafe(written, originalProperties));
throw e;
}
}
+ /**
+ * Builds the result holder, dropping collected URNs that the final
properties still reference (a
+ * later change in the same batch may have re-bound the same key).
+ */
+ private static SecretMaterialsHolder holderOf(
+ List<SecretMaterial> written,
+ List<SecretUrn> replacedUrns,
+ Map<String, String> properties,
+ Map<String, String> originalProperties) {
+ SecretMaterialsHolder holder = new SecretMaterialsHolder();
+ holder.set(List.copyOf(rollbackSafe(written, originalProperties)));
+ holder.setReplacedUrns(
+ replacedUrns.stream()
+ .filter(urn -> !properties.containsValue(urn.toString()))
+ .collect(Collectors.toList()));
Review Comment:
Done in 2c1773525: the collected URNs are now deduplicated and the list is
unmodifiable, matching the materials list in the same holder.
--
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]
