jerryshao commented on code in PR #14559:
URL: https://github.com/apache/gravitino/pull/14559#discussion_r4475753971
##########
core/src/main/java/org/apache/gravitino/domain/DomainDispatcher.java:
##########
@@ -86,29 +95,39 @@ default String[] listDomains(String metalake) throws
NoSuchMetalakeException {
*/
DomainEntity getDomain(String metalake, String name) throws
NoSuchDomainException;
- // TODO: The next PR of #13884 adds the explicit backing tag to createDomain
and
- // createSubdomain, which every create request must name, and adds
updateBackingTag.
-
/**
- * Creates a top-level domain in draft status.
+ * Creates a top-level domain in draft status, backed by a new or an
existing tag.
*
* @param metalake The name of the metalake.
* @param name The name of the domain.
* @param comment The comment of the domain, or null.
* @param properties The properties of the domain, or null.
+ * @param backingTag The backing tag of the domain: the name of the tag to
create, or an existing
+ * tag with an optional value.
+ * @param createBackingTag Whether to create the backing tag as a new tag
without values.
* @return The created domain.
* @throws NoSuchMetalakeException If the metalake does not exist.
* @throws DomainAlreadyExistsException If a domain with the same name
exists.
+ * @throws NoSuchTagException If the existing backing tag to bind does not
exist.
+ * @throws TagAlreadyExistsException If the backing tag to create already
exists.
+ * @throws BackingTagAlreadyBoundException If the binding conflicts with the
backing tag of
+ * another domain.
+ * @throws IllegalArgumentException If the backing tag is null, a value is
given for a tag to
+ * create, or the value is not allowed by the existing tag.
*/
DomainEntity createDomain(
String metalake,
String name,
@Nullable String comment,
- @Nullable Map<String, String> properties)
- throws DomainAlreadyExistsException;
+ @Nullable Map<String, String> properties,
+ TagValue backingTag,
+ boolean createBackingTag)
Review Comment:
I kept `createBackingTag` here to match the public
`DomainOperations.createDomain` and `Domain.createSubdomain` signatures. An
enum only in the dispatcher would require callers to translate the same choice
at the API boundary.
##########
core/src/main/java/org/apache/gravitino/domain/DomainManager.java:
##########
@@ -496,6 +589,44 @@ private static void checkParentName(String name, @Nullable
String parent) {
!Objects.equals(name, parent), "Domain %s cannot be its own parent",
name);
}
+ // Checks that the locked backing tag allows the value and that no other
domain has a binding of
+ // the tag that conflicts with it: a tag has either one binding without a
value, or bindings with
+ // values where each value backs at most one domain.
+ private static void checkBinding(
+ DomainEntity domain, Optional<String> value, LockedDomainState state) {
+ TagEntity tag = lockedBackingTag(state);
+ tag.validateAllowedValue(
+ value.map(v -> TagValue.of(tag.name(), v)).orElseGet(() ->
TagValue.noValue(tag.name())));
+
+ for (DomainEntity bound : state.backingTagBindings()) {
+ if (Objects.equals(bound.id(), domain.id())) {
+ continue;
+ }
+ if (bound.backingTagValue() == null) {
+ throw new BackingTagAlreadyBoundException(
Review Comment:
The class-level Javadoc on `DomainDispatcher` already spells this out in the
paragraph beginning “Every new domain is backed by a tag”. It says that a tag
has either one binding without a value, or bindings with values where each
value backs at most one domain. I kept the rule there so readers can find it
alongside the other domain constraints.
--
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]
