From c1e8162eda4d5259cd503fd4828b4b50ebc90579 Mon Sep 17 00:00:00 2001 From: Mark Czotter Date: Tue, 12 Sep 2023 13:17:00 +0200 Subject: [PATCH] feat(index): support branch name and path aliases A set of name aliases can be specified for a given, existing branch. This works just like the primary path of the branch and can be used to access the branch. Only a single path alias can exist at a given time which points to a single branch document. --- .../revision/RevisionBranchPathAliasTest.java | 117 ++++++++++++++++++ .../index/revision/RevisionBranchingTest.java | 2 +- .../index/revision/BaseRevisionBranching.java | 115 ++++++++++++++--- .../index/revision/RevisionBranch.java | 55 +++++++- .../core/branch/BranchUpdateRequest.java | 33 ++++- .../branch/BranchUpdateRequestBuilder.java | 19 ++- 6 files changed, 309 insertions(+), 32 deletions(-) create mode 100644 commons/com.b2international.index.tests/src/com/b2international/index/revision/RevisionBranchPathAliasTest.java diff --git a/commons/com.b2international.index.tests/src/com/b2international/index/revision/RevisionBranchPathAliasTest.java b/commons/com.b2international.index.tests/src/com/b2international/index/revision/RevisionBranchPathAliasTest.java new file mode 100644 index 00000000000..1baee6543df --- /dev/null +++ b/commons/com.b2international.index.tests/src/com/b2international/index/revision/RevisionBranchPathAliasTest.java @@ -0,0 +1,117 @@ +/* + * Copyright 2023 B2i Healthcare Pte Ltd, http://b2i.sg + * + * Licensed 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 com.b2international.index.revision; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.junit.Assert.*; + +import java.util.Collection; +import java.util.List; + +import org.junit.Test; + +import com.b2international.commons.exceptions.BadRequestException; +import com.b2international.commons.exceptions.NotFoundException; +import com.b2international.index.revision.RevisionFixtures.RevisionData; +import com.google.common.collect.ImmutableSortedSet; + +/** + * @since 9.0 + */ +public class RevisionBranchPathAliasTest extends BaseRevisionIndexTest { + + @Override + protected Collection> getTypes() { + return List.of(RevisionData.class); + } + + @Test + public void createBranchWithAlias() throws Exception { + String branchA = createBranch(MAIN, "a"); + + assertTrue(branching().updateNameAliases(branchA, ImmutableSortedSet.of("b"))); + + // using the path alias the system should be able to get the branch + RevisionBranch doc = getBranch("MAIN/b"); + assertThat(doc.getName()).isEqualTo("a"); + assertThat(doc.getPath()).isEqualTo("MAIN/a"); + assertThat(doc.getNameAliases()).isEqualTo(ImmutableSortedSet.of("b")); + assertThat(doc.getPathAliases()).isEqualTo(ImmutableSortedSet.of("MAIN/b")); + } + + @Test + public void conflictsWithPath() throws Exception { + String branchA = createBranch(MAIN, "a"); + String branchB = createBranch(MAIN, "b"); + + assertThatExceptionOfType(BadRequestException.class) + .isThrownBy(() -> branching().updateNameAliases(branchA, ImmutableSortedSet.of("b"))) + .withMessage("Conflicting path aliases when trying to update nameAliases of 'MAIN/a' to '[b]'"); + } + + @Test + public void conflictsWithPathAlias() throws Exception { + String branchA = createBranch(MAIN, "a"); + String branchB = createBranch(MAIN, "b"); + + assertTrue(branching().updateNameAliases(branchA, ImmutableSortedSet.of("c"))); + + assertThatExceptionOfType(BadRequestException.class) + .isThrownBy(() -> branching().updateNameAliases(branchB, ImmutableSortedSet.of("c"))) + .withMessage("Conflicting path aliases when trying to update nameAliases of 'MAIN/b' to '[c]'"); + } + + @Test + public void nullValue() throws Exception { + String branchA = createBranch(MAIN, "a"); + assertFalse(branching().updateNameAliases(branchA, null)); + } + + @Test + public void clearNameAliases() throws Exception { + String branchA = createBranch(MAIN, "a"); + + // set two alias + assertTrue(branching().updateNameAliases(branchA, ImmutableSortedSet.of("b", "c"))); + + // clear alias array + assertTrue(branching().updateNameAliases(branchA, ImmutableSortedSet.of())); + + // b alias should not return the branch + assertThatExceptionOfType(NotFoundException.class) + .isThrownBy(() -> getBranch("MAIN/b")); + assertThatExceptionOfType(NotFoundException.class) + .isThrownBy(() -> getBranch("MAIN/c")); + } + + @Test + public void removeNameAlias() throws Exception { + String branchA = createBranch(MAIN, "a"); + + assertTrue(branching().updateNameAliases(branchA, ImmutableSortedSet.of("b", "c"))); + + assertTrue(branching().updateNameAliases(branchA, ImmutableSortedSet.of("c"))); + + // b alias should not return the branch + assertThatExceptionOfType(NotFoundException.class) + .isThrownBy(() -> getBranch("MAIN/b")); + + // c alias is still functional + getBranch("MAIN/c"); + } + +} diff --git a/commons/com.b2international.index.tests/src/com/b2international/index/revision/RevisionBranchingTest.java b/commons/com.b2international.index.tests/src/com/b2international/index/revision/RevisionBranchingTest.java index e15c2e88eb1..6546571312e 100644 --- a/commons/com.b2international.index.tests/src/com/b2international/index/revision/RevisionBranchingTest.java +++ b/commons/com.b2international.index.tests/src/com/b2international/index/revision/RevisionBranchingTest.java @@ -286,7 +286,7 @@ public void updateMetadata() throws Exception { private ObjectAssert assertBranchCreate(String branchName) { final String branchPath = branching().createBranch(MAIN, branchName, null, false); - return assertThat(branching().get(branchPath)); + return assertThat(branching().getBranch(branchPath)); } } diff --git a/commons/com.b2international.index/src/com/b2international/index/revision/BaseRevisionBranching.java b/commons/com.b2international.index/src/com/b2international/index/revision/BaseRevisionBranching.java index 44e41375c2e..0da2a34a89a 100644 --- a/commons/com.b2international.index/src/com/b2international/index/revision/BaseRevisionBranching.java +++ b/commons/com.b2international.index/src/com/b2international/index/revision/BaseRevisionBranching.java @@ -22,7 +22,9 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.ReentrantLock; import java.util.function.Consumer; +import java.util.stream.Collectors; +import com.b2international.commons.CompareUtils; import com.b2international.commons.exceptions.*; import com.b2international.commons.options.Metadata; import com.b2international.index.BulkUpdate; @@ -33,11 +35,15 @@ import com.b2international.index.query.Query; import com.b2international.index.query.Query.AfterWhereBuilder; import com.b2international.index.revision.RevisionBranch.BranchState; +import com.b2international.index.revision.RevisionBranch.Fields; import com.google.common.base.Strings; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; -import com.google.common.collect.*; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSortedSet; +import com.google.common.collect.Iterables; +import com.google.common.collect.Sets; /** * @since 6.5 @@ -95,7 +101,7 @@ private T locked(final String lockPath, Callable callable) { } protected void init() { - RevisionBranch mainBranch = get(RevisionBranch.MAIN_PATH); + RevisionBranch mainBranch = get(RevisionBranch.MAIN_PATH, false); if (mainBranch == null) { final long branchId = getMainBranchId(); final long baseTimestamp = getMainBaseTimestamp(); @@ -118,10 +124,17 @@ protected void init() { protected abstract long getMainBranchId(); - public RevisionBranch getBranch(String branchPath) { - final RevisionBranch branch = get(branchPath); + /** + * Retrieves a branch using either its unique branch path or any of the assigned path aliases. + * + * @param branchPathOrAlias + * @return {@link RevisionBranch} instance, never null + * @throws NotFoundException - if the branch does not exist in the system + */ + public RevisionBranch getBranch(String branchPathOrAlias) { + final RevisionBranch branch = get(branchPathOrAlias, true); if (branch == null) { - throw new NotFoundException("Branch", branchPath); + throw new NotFoundException("Branch", branchPathOrAlias); } return branch; } @@ -138,13 +151,30 @@ public RevisionBranch getBranch(long branchId) { /** - * Returns the revision branch for the given branchPath. + * Retrieves a branch using either its unique branch path or any of the assigned path aliases or null the branch does not exist yet. * - * @param branchPath - * @return + * @param branchPathOrAlias + * @return a {@link RevisionBranch} instance or null if not present */ - protected RevisionBranch get(String branchPath) { - return index().read(searcher -> searcher.get(RevisionBranch.class, branchPath)); + protected RevisionBranch get(String branchPathOrAlias, boolean searchPathAliases) { + return index().read(searcher -> { + // for primary paths always use the doc GET method for fastest retrieval + RevisionBranch branch = searcher.get(RevisionBranch.class, branchPathOrAlias); + if (branch != null) { + return branch; + } + + // only perform path alias search if requested + if (!searchPathAliases) { + return null; + } + return Query.select(RevisionBranch.class) + .where(Expressions.exactMatch(Fields.PATH_ALIASES, branchPathOrAlias)) + .limit(1) + .build() + .search(searcher) + .first(); + }); } /** @@ -201,10 +231,6 @@ public T commit(IndexWrite changes) { }); } - protected final String toAbsolutePath(final String parentPath, final String name) { - return parentPath.concat(RevisionBranch.SEPARATOR).concat(name); - } - /** * Creates a new child branch with the given name and metadata under the specified parent branch. * @@ -223,23 +249,24 @@ public final String createBranch(final String parent, final String name, final M if (parentBranch.isDeleted()) { throw new BadRequestException("Cannot create '%s' child branch under deleted '%s' parent.", name, parentBranch.getPath()); } - final String path = toAbsolutePath(parent, name); - RevisionBranch existingBranch = get(path); + // first check if there is an existing branch using the given name + final String path = RevisionBranch.get(parent, name); + RevisionBranch existingBranch = get(path, false); if (!force && existingBranch != null && !existingBranch.isDeleted()) { // throw AlreadyExistsException if exists before trying to enter the sync block throw new AlreadyExistsException("Branch", path); } else { - return create(parentBranch, name, metadata, force); + return doCreateLocked(parentBranch, name, metadata, force); } } - private String create(final RevisionBranch parent, final String name, final Metadata metadata, boolean force) { + private String doCreateLocked(final RevisionBranch parent, final String name, final Metadata metadata, boolean force) { // prevents problematic branch creation from multiple threads, but allows them // to respond back successfully if branch did not exist before creation and it does now final String parentPath = parent.getPath(); return locked(parentPath, () -> { // check again and return if exists, otherwise open the child branch - final RevisionBranch existingBranch = get(toAbsolutePath(parentPath, name)); + final RevisionBranch existingBranch = get(RevisionBranch.get(parentPath, name), false); if (!force && existingBranch != null && !existingBranch.isDeleted()) { return existingBranch.getPath(); } else { @@ -524,7 +551,55 @@ protected final RevisionBranch getBranchFromStore(final AfterWhereBuilder nameAliases) { + RevisionBranch branchToUpdate = getBranch(branchPath); + return commit(writer -> { + RevisionBranch.Builder updated = branchToUpdate.toBuilder(); + boolean changed = updateNameAliases(branchToUpdate, updated, nameAliases); + if (changed) { + writer.put(updated.build()); + } + return changed; + }); + } + + // branch field updaters + + public final boolean updateMetadata(RevisionBranch branchToUpdate, RevisionBranch.Builder updated, Metadata metadata) { + if (metadata == null || Objects.equals(branchToUpdate.metadata(), metadata)) { + return false; + } + + updated.metadata(metadata); + return true; + } + + public final boolean updateNameAliases(RevisionBranch branchToUpdate, RevisionBranch.Builder updated, SortedSet newNameAliases) { + if (newNameAliases == null || Objects.equals(branchToUpdate.getNameAliases(), newNameAliases)) { + return false; + } + // check aliases for collision only if we are not clearing name aliases + if (!CompareUtils.isEmpty(newNameAliases)) { + final Set newPathAliases = newNameAliases.stream().map(nameAlias -> RevisionBranch.get(branchToUpdate.getParentPath(), nameAlias)).collect(Collectors.toSet()); + final long numberOfExistingPaths = search(Query.select(RevisionBranch.class) + .where(Expressions.bool() + .should(Expressions.matchAny(RevisionBranch.Fields.PATH, newPathAliases)) + .should(Expressions.matchAny(RevisionBranch.Fields.PATH_ALIASES, newPathAliases)) + // make sure we exclude the current branch from the results, so it won't collide with itself + .mustNot(Expressions.exactMatch(RevisionBranch.Fields.ID, branchToUpdate.getId())) + .build()) + .limit(0) // hit count only + .build()).getTotal(); + if (numberOfExistingPaths > 0) { + throw new BadRequestException("Conflicting path aliases when trying to update nameAliases of '%s' to '%s'", branchToUpdate.getPath(), newNameAliases); + } + } + + updated.nameAliases(newNameAliases); + return true; } } diff --git a/commons/com.b2international.index/src/com/b2international/index/revision/RevisionBranch.java b/commons/com.b2international.index/src/com/b2international/index/revision/RevisionBranch.java index aa6ef2bb4e4..40d2c389c87 100644 --- a/commons/com.b2international.index/src/com/b2international/index/revision/RevisionBranch.java +++ b/commons/com.b2international.index/src/com/b2international/index/revision/RevisionBranch.java @@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.collect.Maps.newHashMap; +import java.nio.file.Paths; import java.util.*; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -97,9 +98,13 @@ public static enum BranchState { */ public static final class Fields { public static final String ID = "id"; - public static final String PATH = "path"; public static final String NAME = "name"; + public static final String NAME_ALIASES = "nameAliases"; public static final String PARENT_PATH = "parentPath"; + + // derived fields + public static final String PATH = "path"; + public static final String PATH_ALIASES = "pathAliases"; } /** @@ -186,6 +191,7 @@ public static final class Builder { private long id; private String parentPath; private String name; + private SortedSet nameAliases; private boolean deleted; private Metadata metadata; private SortedSet segments; @@ -208,6 +214,11 @@ public Builder name(String name) { return this; } + public Builder nameAliases(SortedSet nameAliases) { + this.nameAliases = nameAliases; + return this; + } + public Builder deleted(boolean deleted) { this.deleted = deleted; return this; @@ -228,15 +239,21 @@ public Builder mergeSources(List mergeSources) { return this; } + // derived fields, empty builder methods required for Jackson deserialization Builder path(String path) { return this; } + Builder pathAliases(SortedSet path) { + return this; + } + public RevisionBranch build() { return new RevisionBranch( id, parentPath, name, + nameAliases, deleted, metadata, segments, @@ -247,10 +264,15 @@ public RevisionBranch build() { private final long id; private final String name; + private final SortedSet nameAliases; + private final String parentPath; @ID private final String path; + + private final SortedSet pathAliases; + private final boolean deleted; private final SortedSet segments; private final List mergeSources; @@ -259,6 +281,7 @@ private RevisionBranch( long id, String parentPath, String name, + SortedSet nameAliases, boolean deleted, Metadata metadata, SortedSet segments, @@ -268,7 +291,9 @@ private RevisionBranch( this.id = id; this.parentPath = parentPath; this.name = name; - this.path = CompareUtils.isEmpty(parentPath) ? name : String.join(SEPARATOR, parentPath, name); + this.nameAliases = nameAliases; + this.path = CompareUtils.isEmpty(parentPath) ? name : get(parentPath, name); + this.pathAliases = nameAliases == null ? null : nameAliases.stream().map(nameAlias -> get(parentPath, nameAlias)).collect(Collectors.toCollection(TreeSet::new)); this.deleted = deleted; this.segments = ImmutableSortedSet.copyOf(segments); this.mergeSources = Collections3.toImmutableList(mergeSources); @@ -291,6 +316,13 @@ public String getPath() { return path; } + /** + * @return the calculated path aliases based on the assigned {@link #getNameAliases() name aliases}. + */ + public SortedSet getPathAliases() { + return pathAliases; + } + /** * Returns the unique path of the parent of this {@link RevisionBranch}. * @@ -309,6 +341,13 @@ public String getName() { return name; } + /** + * @return the assigned name aliases for this {@link RevisionBranch} + */ + public SortedSet getNameAliases() { + return nameAliases; + } + /** * Returns the base timestamp value of this {@link RevisionBranch}. The base timestamp represents the time when this branch has been created, or branched * of from its parent. @@ -474,5 +513,15 @@ public RevisionBranchRef difference(RevisionBranch other) { public RevisionBranch.Builder toBuilder() { return builder(this); } - + + /** + * Similarly to {@link Paths#get(String, String...)} this calculates a valid absolute branch path from the given path segments. + * + * @param pathSegments + * @return a non-null absolute branch path + */ + public static String get(String...pathSegments) { + return String.join(SEPARATOR, pathSegments); + } + } diff --git a/core/com.b2international.snowowl.core/src/com/b2international/snowowl/core/branch/BranchUpdateRequest.java b/core/com.b2international.snowowl.core/src/com/b2international/snowowl/core/branch/BranchUpdateRequest.java index e3fc861c2ac..1024ada3aea 100644 --- a/core/com.b2international.snowowl.core/src/com/b2international/snowowl/core/branch/BranchUpdateRequest.java +++ b/core/com.b2international.snowowl.core/src/com/b2international/snowowl/core/branch/BranchUpdateRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2021 B2i Healthcare Pte Ltd, http://b2i.sg + * Copyright 2011-2023 B2i Healthcare Pte Ltd, http://b2i.sg * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,8 +15,11 @@ */ package com.b2international.snowowl.core.branch; +import java.util.SortedSet; + import com.b2international.commons.options.Metadata; import com.b2international.index.revision.BaseRevisionBranching; +import com.b2international.index.revision.RevisionBranch; import com.b2international.snowowl.core.authorization.AccessControl; import com.b2international.snowowl.core.domain.RepositoryContext; import com.b2international.snowowl.core.identity.Permission; @@ -27,9 +30,14 @@ */ public final class BranchUpdateRequest extends BranchBaseRequest implements AccessControl { + private static final long serialVersionUID = 1L; + @JsonProperty private Metadata metadata; + @JsonProperty + private SortedSet nameAliases; + BranchUpdateRequest(String branchPath) { super(branchPath); } @@ -38,13 +46,26 @@ void setMetadata(Metadata metadata) { this.metadata = metadata; } + void setNameAliases(SortedSet nameAliases) { + this.nameAliases = nameAliases; + } + @Override public Boolean execute(RepositoryContext context) { - if (metadata != null) { - context.service(BaseRevisionBranching.class).updateMetadata(getBranchPath(), metadata); - return Boolean.TRUE; - } - return Boolean.FALSE; + BaseRevisionBranching branching = context.service(BaseRevisionBranching.class); + RevisionBranch branch = branching.getBranch(getBranchPath()); + return branching.commit(writer -> { + boolean changed = false; + RevisionBranch.Builder updated = branch.toBuilder(); + changed |= branching.updateMetadata(branch, updated, metadata); + changed |= branching.updateNameAliases(branch, updated, nameAliases); + + if (changed) { + writer.put(updated.build()); + } + + return changed; + }); } @Override diff --git a/core/com.b2international.snowowl.core/src/com/b2international/snowowl/core/branch/BranchUpdateRequestBuilder.java b/core/com.b2international.snowowl.core/src/com/b2international/snowowl/core/branch/BranchUpdateRequestBuilder.java index 94a9213f76b..2adf35a8fa7 100644 --- a/core/com.b2international.snowowl.core/src/com/b2international/snowowl/core/branch/BranchUpdateRequestBuilder.java +++ b/core/com.b2international.snowowl.core/src/com/b2international/snowowl/core/branch/BranchUpdateRequestBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2017 B2i Healthcare Pte Ltd, http://b2i.sg + * Copyright 2011-2023 B2i Healthcare Pte Ltd, http://b2i.sg * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,11 +15,14 @@ */ package com.b2international.snowowl.core.branch; +import java.util.SortedSet; + import com.b2international.commons.options.Metadata; import com.b2international.snowowl.core.domain.RepositoryContext; import com.b2international.snowowl.core.events.BaseRequestBuilder; import com.b2international.snowowl.core.events.Request; import com.b2international.snowowl.core.request.RepositoryRequestBuilder; +import com.google.common.collect.ImmutableSortedSet; /** * @since 5.0 @@ -28,13 +31,14 @@ public final class BranchUpdateRequestBuilder extends BaseRequestBuilder nameAliases; BranchUpdateRequestBuilder(String branchPath) { this.branchPath = branchPath; } /** - * Update (override) the current {@link Metadata} with the specified {@link Metadata}. + * Update (override) the current {@link Metadata} with the specified {@link Metadata}. To clear metadata, specify an empty {@link Metadata} object. * @param metadata * @return */ @@ -43,10 +47,21 @@ public BranchUpdateRequestBuilder setMetadata(Metadata metadata) { return getSelf(); } + /** + * Update (override) the current set of name aliases assigned to the selected branch. To clear name aliases, specify an empty collection. + * @param nameAliases + * @return + */ + public BranchUpdateRequestBuilder setNameAliases(Iterable nameAliases) { + this.nameAliases = nameAliases == null ? null : ImmutableSortedSet.copyOf(nameAliases); + return getSelf(); + } + @Override protected Request doBuild() { final BranchUpdateRequest req = new BranchUpdateRequest(branchPath); req.setMetadata(metadata); + req.setNameAliases(nameAliases); return req; }