dennishuo commented on code in PR #3237: URL: https://github.com/apache/polaris/pull/3237#discussion_r2609781375
########## persistence/nosql/persistence/metastore/src/main/java/org/apache/polaris/persistence/nosql/metastore/privs/GrantTriplet.java: ########## @@ -0,0 +1,57 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.persistence.nosql.metastore.privs; + +import static com.google.common.base.Preconditions.checkArgument; + +import org.apache.polaris.core.entity.PolarisEntityCore; + +/** + * Represents the triplet of catalog-ID, entity-ID and type-code plus a reverse-or-key marker. + * String representations of this type are used as ACL names and "role" names. Review Comment: Looks like there's an implicit encoding convention here regarding the `r/` and `d/` convention. Could use javadoc examples here or else a javadoc link to wherever else the convention is defined other than just in the code's parsing logic in here. ########## persistence/nosql/persistence/metastore/src/main/java/org/apache/polaris/persistence/nosql/metastore/ContentIdentifier.java: ########## @@ -0,0 +1,168 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.persistence.nosql.metastore; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkState; + +import com.fasterxml.jackson.annotation.JsonValue; +import com.google.errorprone.annotations.CanIgnoreReturnValue; +import java.util.ArrayList; +import java.util.List; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.polaris.immutables.PolarisImmutable; +import org.apache.polaris.persistence.nosql.api.index.IndexKey; +import org.apache.polaris.service.types.PolicyIdentifier; +import org.immutables.value.Value; + [email protected](underrideToString = "asDotDelimitedString") +@PolarisImmutable +public interface ContentIdentifier { + @Value.Parameter + @JsonValue + List<String> elements(); + + static ContentIdentifier identifier(List<String> elements) { + return ImmutableContentIdentifier.of(elements); + } + + static ContentIdentifier identifier(String[] namespace, String name) { + return ImmutableContentIdentifier.builder().addElements(namespace).addElements(name).build(); + } + + static ContentIdentifier identifier(String... elements) { + return ImmutableContentIdentifier.of(List.of(elements)); + } + + static ContentIdentifier identifierFor(PolicyIdentifier identifier) { + return identifier(identifier.getNamespace().levels(), identifier.getName()); + } + + static ContentIdentifier identifierFor(TableIdentifier identifier) { + return identifier(identifier.namespace().levels(), identifier.name()); + } + + static ContentIdentifier identifierFor(Namespace namespace) { + return identifier(namespace.levels()); + } + + static ContentIdentifier identifierFromLocationString(String locationString) { + var builder = builder(); + var len = locationString.length(); + var off = -1; + for (var i = 0; i < len; i++) { + var c = locationString.charAt(i); + checkArgument(c >= ' ', "Control characters are forbidden in locations"); + if (c == '/' || c == '\\') { Review Comment: Is this for linux vs windows path separators? Can we just use `java.nio.Path.of(...).iterator()` instead? ########## persistence/nosql/persistence/metastore/src/main/java/org/apache/polaris/persistence/nosql/metastore/NoSqlMetaStore.java: ########## @@ -0,0 +1,1347 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.persistence.nosql.metastore; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkState; +import static java.lang.String.format; +import static java.util.Objects.requireNonNull; +import static org.apache.polaris.core.entity.PolarisEntityConstants.ENTITY_BASE_LOCATION; +import static org.apache.polaris.core.persistence.dao.entity.BaseResult.ReturnStatus.ENTITY_ALREADY_EXISTS; +import static org.apache.polaris.core.persistence.dao.entity.BaseResult.ReturnStatus.ENTITY_NOT_FOUND; +import static org.apache.polaris.persistence.nosql.api.index.IndexContainer.newUpdatableIndex; +import static org.apache.polaris.persistence.nosql.api.index.IndexKey.INDEX_KEY_SERIALIZER; +import static org.apache.polaris.persistence.nosql.api.obj.ObjRef.OBJ_REF_SERIALIZER; +import static org.apache.polaris.persistence.nosql.api.obj.ObjRef.objRef; +import static org.apache.polaris.persistence.nosql.coretypes.catalog.CatalogStateObj.CATALOG_STATE_REF_NAME_PATTERN; +import static org.apache.polaris.persistence.nosql.coretypes.catalog.EntityIdSet.ENTITY_ID_SET_SERIALIZER; +import static org.apache.polaris.persistence.nosql.coretypes.mapping.EntityObjMappings.containerTypeForEntityType; +import static org.apache.polaris.persistence.nosql.coretypes.mapping.EntityObjMappings.filterIsEntityType; +import static org.apache.polaris.persistence.nosql.coretypes.mapping.EntityObjMappings.isCatalogContent; +import static org.apache.polaris.persistence.nosql.coretypes.mapping.EntityObjMappings.mapToEntity; +import static org.apache.polaris.persistence.nosql.coretypes.mapping.EntityObjMappings.objTypeForPolarisTypeForFiltering; +import static org.apache.polaris.persistence.nosql.coretypes.mapping.EntityObjMappings.referenceName; +import static org.apache.polaris.persistence.nosql.coretypes.realm.PolicyMapping.POLICY_MAPPING_SERIALIZER; +import static org.apache.polaris.persistence.nosql.coretypes.realm.PolicyMappingsObj.POLICY_MAPPINGS_REF_NAME; +import static org.apache.polaris.persistence.nosql.coretypes.realm.PolicyMappingsObj.PolicyMappingKey.fromIndexKey; +import static org.apache.polaris.persistence.nosql.coretypes.realm.RootObj.ROOT_REF_NAME; +import static org.apache.polaris.persistence.nosql.coretypes.refs.References.catalogReferenceNames; +import static org.apache.polaris.persistence.nosql.metastore.ContentIdentifier.identifierFromLocationString; +import static org.apache.polaris.persistence.nosql.metastore.ContentIdentifier.indexKeyToIdentifier; +import static org.apache.polaris.persistence.nosql.metastore.indexaccess.MemoizedIndexedAccess.newMemoizedIndexedAccess; +import static org.apache.polaris.persistence.nosql.metastore.mutation.EntityUpdate.Operation.CREATE; +import static org.apache.polaris.persistence.nosql.metastore.mutation.EntityUpdate.Operation.UPDATE; +import static org.apache.polaris.persistence.nosql.metastore.mutation.UpdateKeyForCatalogAndEntityType.updateKeyForCatalogAndEntityType; + +import com.google.common.collect.Streams; +import jakarta.annotation.Nonnull; +import jakarta.annotation.Nullable; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.function.Function; +import java.util.function.Predicate; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import org.apache.polaris.core.PolarisDiagnostics; +import org.apache.polaris.core.entity.AsyncTaskType; +import org.apache.polaris.core.entity.LocationBasedEntity; +import org.apache.polaris.core.entity.PolarisBaseEntity; +import org.apache.polaris.core.entity.PolarisEntity; +import org.apache.polaris.core.entity.PolarisEntityConstants; +import org.apache.polaris.core.entity.PolarisEntityCore; +import org.apache.polaris.core.entity.PolarisEntitySubType; +import org.apache.polaris.core.entity.PolarisEntityType; +import org.apache.polaris.core.entity.PolarisGrantRecord; +import org.apache.polaris.core.entity.PolarisPrincipalSecrets; +import org.apache.polaris.core.entity.PolarisPrivilege; +import org.apache.polaris.core.entity.PolarisTaskConstants; +import org.apache.polaris.core.persistence.BaseMetaStoreManager; +import org.apache.polaris.core.persistence.PolarisObjectMapperUtil; +import org.apache.polaris.core.persistence.bootstrap.RootCredentialsSet; +import org.apache.polaris.core.persistence.dao.entity.CreateCatalogResult; +import org.apache.polaris.core.persistence.dao.entity.CreatePrincipalResult; +import org.apache.polaris.core.persistence.dao.entity.DropEntityResult; +import org.apache.polaris.core.persistence.dao.entity.EntitiesResult; +import org.apache.polaris.core.persistence.dao.entity.EntityResult; +import org.apache.polaris.core.persistence.dao.entity.EntityWithPath; +import org.apache.polaris.core.persistence.dao.entity.LoadGrantsResult; +import org.apache.polaris.core.persistence.dao.entity.LoadPolicyMappingsResult; +import org.apache.polaris.core.persistence.dao.entity.PolicyAttachmentResult; +import org.apache.polaris.core.persistence.pagination.Page; +import org.apache.polaris.core.persistence.pagination.PageToken; +import org.apache.polaris.core.policy.PolarisPolicyMappingRecord; +import org.apache.polaris.core.policy.PolicyMappingUtil; +import org.apache.polaris.core.policy.PolicyType; +import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; +import org.apache.polaris.core.storage.PolarisStorageIntegration; +import org.apache.polaris.core.storage.PolarisStorageIntegrationProvider; +import org.apache.polaris.core.storage.StorageLocation; +import org.apache.polaris.persistence.nosql.api.Persistence; +import org.apache.polaris.persistence.nosql.api.index.Index; +import org.apache.polaris.persistence.nosql.api.index.IndexKey; +import org.apache.polaris.persistence.nosql.api.obj.ObjRef; +import org.apache.polaris.persistence.nosql.api.obj.ObjTypes; +import org.apache.polaris.persistence.nosql.authz.api.Privilege; +import org.apache.polaris.persistence.nosql.authz.api.PrivilegeSet; +import org.apache.polaris.persistence.nosql.authz.api.Privileges; +import org.apache.polaris.persistence.nosql.coretypes.ContainerObj; +import org.apache.polaris.persistence.nosql.coretypes.ObjBase; +import org.apache.polaris.persistence.nosql.coretypes.acl.AclObj; +import org.apache.polaris.persistence.nosql.coretypes.catalog.CatalogObj; +import org.apache.polaris.persistence.nosql.coretypes.catalog.CatalogRoleObj; +import org.apache.polaris.persistence.nosql.coretypes.catalog.CatalogRolesObj; +import org.apache.polaris.persistence.nosql.coretypes.catalog.CatalogStateObj; +import org.apache.polaris.persistence.nosql.coretypes.catalog.CatalogsObj; +import org.apache.polaris.persistence.nosql.coretypes.content.ContentObj; +import org.apache.polaris.persistence.nosql.coretypes.mapping.EntityObjMappings; +import org.apache.polaris.persistence.nosql.coretypes.principals.PrincipalObj; +import org.apache.polaris.persistence.nosql.coretypes.principals.PrincipalsObj; +import org.apache.polaris.persistence.nosql.coretypes.realm.PolicyMapping; +import org.apache.polaris.persistence.nosql.coretypes.realm.PolicyMappingsObj; +import org.apache.polaris.persistence.nosql.coretypes.realm.RootObj; +import org.apache.polaris.persistence.nosql.metastore.committers.CatalogChangeCommitterWrapper; +import org.apache.polaris.persistence.nosql.metastore.committers.ChangeCommitter; +import org.apache.polaris.persistence.nosql.metastore.committers.ChangeCommitterWrapper; +import org.apache.polaris.persistence.nosql.metastore.committers.ChangeResult; +import org.apache.polaris.persistence.nosql.metastore.indexaccess.IndexedContainerAccess; +import org.apache.polaris.persistence.nosql.metastore.indexaccess.MemoizedIndexedAccess; +import org.apache.polaris.persistence.nosql.metastore.mutation.EntityUpdate; +import org.apache.polaris.persistence.nosql.metastore.mutation.GrantsMutation; +import org.apache.polaris.persistence.nosql.metastore.mutation.MutationAttempt; +import org.apache.polaris.persistence.nosql.metastore.mutation.MutationAttemptRoot; +import org.apache.polaris.persistence.nosql.metastore.mutation.MutationResults; +import org.apache.polaris.persistence.nosql.metastore.mutation.PolicyMutation; +import org.apache.polaris.persistence.nosql.metastore.mutation.PrincipalMutations; +import org.apache.polaris.persistence.nosql.metastore.mutation.PrincipalMutations.UpdateSecrets.SecretsUpdater; +import org.apache.polaris.persistence.nosql.metastore.mutation.UpdateKeyForCatalogAndEntityType; +import org.apache.polaris.persistence.nosql.metastore.privs.GrantTriplet; +import org.apache.polaris.persistence.nosql.metastore.privs.SecurableAndGrantee; +import org.apache.polaris.persistence.nosql.metastore.privs.SecurableGranteePrivilegeTuple; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +class NoSqlMetaStore extends NonFunctionalBasePersistence { + private static final Logger LOGGER = LoggerFactory.getLogger(NoSqlMetaStore.class); + + private final Persistence persistence; + private final Privileges privileges; + private final PolarisStorageIntegrationProvider storageIntegrationProvider; + private final MemoizedIndexedAccess memoizedIndexedAccess; + private final PolarisDiagnostics diagnostics; + + NoSqlMetaStore( + Persistence persistence, + Privileges privileges, + PolarisStorageIntegrationProvider storageIntegrationProvider, + PolarisDiagnostics diagnostics) { + this.persistence = persistence; + this.privileges = privileges; + this.storageIntegrationProvider = storageIntegrationProvider; + this.memoizedIndexedAccess = newMemoizedIndexedAccess(persistence); + this.diagnostics = diagnostics; + } + + <REF_OBJ extends ContainerObj, RESULT> RESULT performChange( + @Nonnull PolarisEntityType entityType, + @Nonnull Class<REF_OBJ> referencedObjType, + @Nonnull Class<RESULT> resultType, + long catalogStableId, + @Nonnull ChangeCommitter<REF_OBJ, RESULT> changeCommitter) { + try { + var committer = + persistence + .createCommitter( + referenceName(entityType, catalogStableId), referencedObjType, resultType) + .synchronizingLocally(); + var commitRetryable = new ChangeCommitterWrapper<>(changeCommitter, entityType); + return committer.commitRuntimeException(commitRetryable).orElseThrow(); + } finally { + memoizedIndexedAccess.invalidateIndexedAccess(catalogStableId, entityType.getCode()); + } + } + + long generateNewId() { + return persistence.generateId(); + } + + void initializeCatalogsIfNecessary() { + memoizedIndexedAccess + .indexedAccess(0, PolarisEntityType.CATALOG.getCode()) + .nameIndex() + .ifPresent( + names -> + persistence + .bucketizedBulkFetches( + Streams.stream(names).filter(Objects::nonNull).map(Map.Entry::getValue), + CatalogObj.class) + .filter(Objects::nonNull) + .forEach( + catalogObj -> { + LOGGER.debug("Initializing catalog {} if necessary", catalogObj.name()); + initializeCatalogIfNecessary(persistence, catalogObj); + })); + } + + CreateCatalogResult createCatalog( + PolarisBaseEntity catalog, List<PolarisBaseEntity> principalRoles) { + checkArgument(catalog != null && catalog.getType() == PolarisEntityType.CATALOG); + + LOGGER.debug("create catalog #{} '{}'", catalog.getId(), catalog.getName()); + + return performChange( + PolarisEntityType.CATALOG, + CatalogsObj.class, + CreateCatalogResult.class, + 0L, + ((state, ref, byName, byId) -> { + var nameKey = IndexKey.key(catalog.getName()); + var idKey = IndexKey.key(catalog.getId()); + + // check if that catalog has already been created + var existing = byName.get(nameKey); + var persistence = state.persistence(); + var catalogObj = existing != null ? persistence.fetch(existing, CatalogObj.class) : null; + + // if found, probably a retry, simply return the previously created catalog + // TODO not sure how a "retry" could happen with the same ID though (see + // PolarisMetaStoreManagerImpl.createCatalog())... + if (catalogObj != null && catalogObj.stableId() != catalog.getId()) { + // A catalog with the same name already exists (different ID) + return new ChangeResult.NoChange<>( + new CreateCatalogResult(ENTITY_ALREADY_EXISTS, null)); + } + if (catalogObj == null) { + catalogObj = + EntityObjMappings.<CatalogObj, CatalogObj.Builder>mapToObj( + catalog, Optional.empty()) + .id(persistence.generateId()) + .build(); + state.writeOrReplace("catalog", catalogObj); + } + + initializeCatalogIfNecessary(persistence, catalogObj); + + checkState(!byId.contains(idKey), "Catalog ID %s already used", catalog.getId()); + + // 'persistStorageIntegrationIfNeeded' is a no-op in all implementations ?!?!? + // persistStorageIntegrationIfNeeded(callCtx, catalog, integration); + + var catalogAdminRoleObj = + createCatalogRoleIdempotent( + catalogObj, + persistence.generateId(), + PolarisEntityConstants.getNameOfCatalogAdminRole()); + + var catalogAdminRole = mapToEntity(catalogAdminRoleObj, catalogObj.stableId()); + + var grants = new ArrayList<SecurableGranteePrivilegeTuple>(); + + // grant the catalog admin role access-management on the catalog + grants.add( + new SecurableGranteePrivilegeTuple( + catalog, catalogAdminRole, PolarisPrivilege.CATALOG_MANAGE_ACCESS)); + // grant the catalog admin role metadata-management on the catalog; this one is revocable + grants.add( + new SecurableGranteePrivilegeTuple( + catalog, catalogAdminRole, PolarisPrivilege.CATALOG_MANAGE_METADATA)); + + var effRoles = + principalRoles.isEmpty() + ? List.of( + requireNonNull( + lookupEntityByName( + 0L, + 0L, + PolarisEntityType.PRINCIPAL_ROLE.getCode(), + PolarisEntityConstants.getNameOfPrincipalServiceAdminRole()))) + : principalRoles; + + for (PolarisBaseEntity effRole : effRoles) { + grants.add( + new SecurableGranteePrivilegeTuple( + catalogAdminRole, effRole, PolarisPrivilege.CATALOG_ROLE_USAGE)); + } + + persistGrantsOrRevokes(true, grants.toArray(SecurableGranteePrivilegeTuple[]::new)); + + byName.put(nameKey, objRef(catalogObj)); + byId.put(idKey, nameKey); + + if (existing == null) { + // created + return new ChangeResult.CommitChange<>( + new CreateCatalogResult(catalog, catalogAdminRole)); + } + // retry + return new ChangeResult.NoChange<>(new CreateCatalogResult(ENTITY_ALREADY_EXISTS, null)); Review Comment: I'm not sure this fallthrough should exist; if returning `ENTITY_ALREADY_EXISTS` it's interpreted by the upper stack to mean the same *name* is already occupied but that this isn't actually an idempotent retry attempt (e.g. it had a different id). Looks like line 232 already captures that case. If we reach this line does it mean we're accruing a bunch of updates into the updatable indexes that we'll just discard when it comes time to "commit"? If we want consistent behavior with the other metastore impls we would basically need a way to return `NoChange` but with a *success* `CreateCatalogResult` maybe in an else branch on line 230 (`else if (catalogObj.stableId() == catalog.getId())`). It's possible the idempotent-retry scenario that code is trying to catch isn't actually possible anymore though, not sure. I *think* the `checkState(!byId.contains(idKey` on line 246 means that this line wouldn't actually represent stableId == id though, in which case reaching this line should maybe be an IllegalStateException indicating some kind of corrupted state? Unless I'm misunderstanding the state of the index. Basically we'd reach here if the `byName` already exists, and `catalogObj.stableId() != catalog.getId()`, but somehow `catalogObj == null` so that line 230 doesn't already exit early. Is it expected for the byName index to not be atomic with what's visible in `persistence.fetch`? I think all these subtleties could use some code comments to clarify if this is working as intended. ########## persistence/nosql/persistence/metastore/src/main/java/org/apache/polaris/persistence/nosql/metastore/ContentIdentifier.java: ########## @@ -0,0 +1,168 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.persistence.nosql.metastore; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkState; + +import com.fasterxml.jackson.annotation.JsonValue; +import com.google.errorprone.annotations.CanIgnoreReturnValue; +import java.util.ArrayList; +import java.util.List; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.polaris.immutables.PolarisImmutable; +import org.apache.polaris.persistence.nosql.api.index.IndexKey; +import org.apache.polaris.service.types.PolicyIdentifier; +import org.immutables.value.Value; + [email protected](underrideToString = "asDotDelimitedString") +@PolarisImmutable +public interface ContentIdentifier { + @Value.Parameter + @JsonValue + List<String> elements(); + + static ContentIdentifier identifier(List<String> elements) { + return ImmutableContentIdentifier.of(elements); + } + + static ContentIdentifier identifier(String[] namespace, String name) { + return ImmutableContentIdentifier.builder().addElements(namespace).addElements(name).build(); + } + + static ContentIdentifier identifier(String... elements) { + return ImmutableContentIdentifier.of(List.of(elements)); + } + + static ContentIdentifier identifierFor(PolicyIdentifier identifier) { + return identifier(identifier.getNamespace().levels(), identifier.getName()); + } + + static ContentIdentifier identifierFor(TableIdentifier identifier) { + return identifier(identifier.namespace().levels(), identifier.name()); + } + + static ContentIdentifier identifierFor(Namespace namespace) { + return identifier(namespace.levels()); + } + + static ContentIdentifier identifierFromLocationString(String locationString) { Review Comment: All the other factory methods are mostly self-explanatory, but this one could use some javadocs explaining what `locationString` is, expected format, etc. ########## persistence/nosql/persistence/metastore/src/main/java/org/apache/polaris/persistence/nosql/metastore/ContentIdentifier.java: ########## @@ -0,0 +1,168 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.persistence.nosql.metastore; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkState; + +import com.fasterxml.jackson.annotation.JsonValue; +import com.google.errorprone.annotations.CanIgnoreReturnValue; +import java.util.ArrayList; +import java.util.List; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.polaris.immutables.PolarisImmutable; +import org.apache.polaris.persistence.nosql.api.index.IndexKey; +import org.apache.polaris.service.types.PolicyIdentifier; +import org.immutables.value.Value; + [email protected](underrideToString = "asDotDelimitedString") +@PolarisImmutable +public interface ContentIdentifier { + @Value.Parameter + @JsonValue + List<String> elements(); + + static ContentIdentifier identifier(List<String> elements) { + return ImmutableContentIdentifier.of(elements); + } + + static ContentIdentifier identifier(String[] namespace, String name) { + return ImmutableContentIdentifier.builder().addElements(namespace).addElements(name).build(); + } + + static ContentIdentifier identifier(String... elements) { + return ImmutableContentIdentifier.of(List.of(elements)); + } + + static ContentIdentifier identifierFor(PolicyIdentifier identifier) { + return identifier(identifier.getNamespace().levels(), identifier.getName()); + } + + static ContentIdentifier identifierFor(TableIdentifier identifier) { + return identifier(identifier.namespace().levels(), identifier.name()); + } + + static ContentIdentifier identifierFor(Namespace namespace) { + return identifier(namespace.levels()); + } + + static ContentIdentifier identifierFromLocationString(String locationString) { + var builder = builder(); + var len = locationString.length(); + var off = -1; + for (var i = 0; i < len; i++) { + var c = locationString.charAt(i); + checkArgument(c >= ' ', "Control characters are forbidden in locations"); Review Comment: Is this really intended to support all chars with ascii codepoint > ' ' or is there a more intuitive regex? In particular, though I don't know all ascii codes offhand, a cursory search seems to indicate for example that `127` is the "Delete control character" even though it comes after all the basic symbols and letters. Maybe `Character.isJavaIdentifierPart` will help provide a better standard constrained set while also making it easier to reason about when reading the code. ########## persistence/nosql/persistence/metastore/src/main/java/org/apache/polaris/persistence/nosql/metastore/mutation/MutationResults.java: ########## @@ -0,0 +1,148 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.persistence.nosql.metastore.mutation; + +import static org.apache.polaris.core.persistence.dao.entity.BaseResult.ReturnStatus.SUCCESS; + +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; +import java.util.stream.Collectors; +import org.apache.polaris.core.entity.PolarisBaseEntity; +import org.apache.polaris.core.persistence.dao.entity.BaseResult; +import org.apache.polaris.core.persistence.dao.entity.DropEntityResult; +import org.apache.polaris.core.persistence.dao.entity.EntityResult; +import org.apache.polaris.persistence.nosql.api.index.IndexKey; +import org.apache.polaris.persistence.nosql.metastore.privs.GrantTriplet; + +public final class MutationResults { + private final List<BaseResult> results; + // TODO populate and process 'aclsToRemove' + private final List<GrantTriplet> aclsToRemove; + private final List<PolarisBaseEntity> droppedEntities; + private final List<IndexKey> policyIndexKeysToRemove = new ArrayList<>(); + + boolean anyChange; + boolean hardFailure; + + private MutationResults( + List<BaseResult> results, + List<GrantTriplet> aclsToRemove, + List<PolarisBaseEntity> droppedEntities) { + this.results = results; + this.aclsToRemove = aclsToRemove; + this.droppedEntities = droppedEntities; + } + + MutationResults(BaseResult single) { + this(List.of(single), List.of(), List.of()); + } + + static MutationResults singleEntityResult(BaseResult.ReturnStatus returnStatus) { + return new MutationResults(new EntityResult(returnStatus, null)); + } + + static MutationResults singleEntityResult(PolarisBaseEntity entity) { + return new MutationResults(new EntityResult(entity)); + } + + static MutationResults newMutableMutationResults() { + return new MutationResults(new ArrayList<>(), new ArrayList<>(), new ArrayList<>()); + } + + public List<BaseResult> results() { + return results; + } + + public List<GrantTriplet> aclsToRemove() { + return aclsToRemove; + } + + public List<PolarisBaseEntity> droppedEntities() { + return droppedEntities; + } + + public List<IndexKey> policyIndexKeysToRemove() { + return policyIndexKeysToRemove; + } + + void addPolicyIndexKeyToRemove(IndexKey indexKey) { + policyIndexKeysToRemove.add(indexKey); + } + + void entityResult(PolarisBaseEntity entity) { + add(new EntityResult(entity)); + anyChange = true; + } + + void entityResultNoChange(PolarisBaseEntity entity) { Review Comment: Could use some javadoc comments explaining the difference between `entityResultNoChange` and `unchangedEntityResult` -- 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]
