-
Notifications
You must be signed in to change notification settings - Fork 116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ability to update the repository description #7376
Changes from 8 commits
9fa4262
80da4c5
2387fc2
a418a4c
57f88cd
73794cf
103d3d9
8c9d449
ca25275
db71dbc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,8 +45,10 @@ | |
import org.junit.jupiter.params.ParameterizedTest; | ||
import org.junit.jupiter.params.provider.ValueSource; | ||
import org.projectnessie.versioned.storage.common.exceptions.RefNotFoundException; | ||
import org.projectnessie.versioned.storage.common.exceptions.RetryTimeoutException; | ||
import org.projectnessie.versioned.storage.common.logic.CommitLogic; | ||
import org.projectnessie.versioned.storage.common.logic.CreateCommit; | ||
import org.projectnessie.versioned.storage.common.logic.ImmutableRepositoryDescription; | ||
import org.projectnessie.versioned.storage.common.logic.InternalRef; | ||
import org.projectnessie.versioned.storage.common.logic.ReferenceLogic; | ||
import org.projectnessie.versioned.storage.common.logic.RepositoryDescription; | ||
|
@@ -234,4 +236,32 @@ public void internalRefsAndDefaultBranch(String defaultBranchName) { | |
soft.assertThat(newHashSet(referenceLogic.queryReferences(referencesQuery()))) | ||
.isEqualTo(refQuery); | ||
} | ||
|
||
@Test | ||
public void updateRepositoryDescription() throws RetryTimeoutException { | ||
RepositoryLogic repositoryLogic = repositoryLogic(persist); | ||
|
||
repositoryLogic.initialize("main"); | ||
|
||
RepositoryDescription initial = requireNonNull(repositoryLogic.fetchRepositoryDescription()); | ||
|
||
RepositoryDescription updated = | ||
RepositoryDescription.builder() | ||
.putProperties("updated", "true") | ||
// read-only properties, should be ignored | ||
.defaultBranchName("main2") | ||
.oldestPossibleCommitTime(Instant.ofEpochSecond(12345)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This and the below setting are actually read-only attributes. |
||
.repositoryCreatedTime(Instant.ofEpochSecond(456789)) | ||
.build(); | ||
|
||
RepositoryDescription previous = repositoryLogic.updateRepositoryDescription(updated); | ||
|
||
soft.assertThat(previous).isEqualTo(initial); | ||
soft.assertThat(repositoryLogic.fetchRepositoryDescription()) | ||
.isEqualTo( | ||
ImmutableRepositoryDescription.builder() | ||
.from(initial) | ||
.putProperties("updated", "true") | ||
.build()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ | |
|
||
import static java.util.Collections.emptyList; | ||
import static java.util.Objects.requireNonNull; | ||
import static org.projectnessie.versioned.storage.common.logic.CommitRetry.commitRetry; | ||
import static org.projectnessie.versioned.storage.common.logic.CreateCommit.Add.commitAdd; | ||
import static org.projectnessie.versioned.storage.common.logic.CreateCommit.newCommitBuilder; | ||
import static org.projectnessie.versioned.storage.common.logic.InternalRef.KEY_REPO_DESCRIPTION; | ||
|
@@ -38,12 +39,16 @@ | |
import javax.annotation.Nonnull; | ||
import javax.annotation.Nullable; | ||
import org.projectnessie.versioned.storage.common.exceptions.CommitConflictException; | ||
import org.projectnessie.versioned.storage.common.exceptions.CommitWrappedException; | ||
import org.projectnessie.versioned.storage.common.exceptions.ObjNotFoundException; | ||
import org.projectnessie.versioned.storage.common.exceptions.ObjTooLargeException; | ||
import org.projectnessie.versioned.storage.common.exceptions.RefAlreadyExistsException; | ||
import org.projectnessie.versioned.storage.common.exceptions.RefConditionFailedException; | ||
import org.projectnessie.versioned.storage.common.exceptions.RefNotFoundException; | ||
import org.projectnessie.versioned.storage.common.exceptions.RetryTimeoutException; | ||
import org.projectnessie.versioned.storage.common.indexes.StoreIndex; | ||
import org.projectnessie.versioned.storage.common.indexes.StoreIndexElement; | ||
import org.projectnessie.versioned.storage.common.logic.CommitRetry.RetryException; | ||
import org.projectnessie.versioned.storage.common.logic.StringLogic.StringValue; | ||
import org.projectnessie.versioned.storage.common.objtypes.CommitObj; | ||
import org.projectnessie.versioned.storage.common.objtypes.CommitOp; | ||
|
@@ -135,20 +140,28 @@ public RepositoryDescription fetchRepositoryDescription() { | |
requireNonNull( | ||
op.value(), "Commit operation for repository description has no value")); | ||
|
||
return readRepositoryDescription(value); | ||
return deserialize(value); | ||
} catch (ObjNotFoundException e) { | ||
return null; | ||
} | ||
} | ||
|
||
private RepositoryDescription readRepositoryDescription(StringValue value) { | ||
private static RepositoryDescription deserialize(StringValue value) { | ||
try { | ||
return SHARED_OBJECT_MAPPER.readValue(value.completeValue(), RepositoryDescription.class); | ||
} catch (ObjNotFoundException | IOException e) { | ||
throw new RuntimeException(e); | ||
} | ||
} | ||
|
||
private static byte[] serialize(RepositoryDescription repositoryDescription) { | ||
try { | ||
return SHARED_OBJECT_MAPPER.writeValueAsBytes(repositoryDescription); | ||
} catch (IOException e) { | ||
throw new RuntimeException(e); | ||
} | ||
} | ||
|
||
private void addRepositoryDescription( | ||
CreateCommit.Builder b, | ||
Consumer<RepositoryDescription.Builder> repositoryDescription, | ||
|
@@ -169,14 +182,60 @@ private void addRepositoryDescription( | |
null, | ||
"application/json", | ||
SHARED_OBJECT_MAPPER.writeValueAsBytes(repoDesc.build())); | ||
// can safely ignore the ID returned from storeObj() - it's fine, if the obj already exists | ||
// can safely ignore the response from storeObj() - it's fine, if the obj already exists | ||
persist.storeObj(string); | ||
b.addAdds(commitAdd(KEY_REPO_DESCRIPTION, 0, requireNonNull(string.id()), null, null)); | ||
} catch (ObjTooLargeException | ObjNotFoundException | IOException e) { | ||
throw new RuntimeException(e); | ||
} | ||
} | ||
|
||
@Override | ||
public RepositoryDescription updateRepositoryDescription(RepositoryDescription newDescription) | ||
throws RetryTimeoutException { | ||
// prevent modification of read-only attributes | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This came to my mind after @dimas-b suggestion to introduce an option in quarkus-cli to allow end users to modify the repo description. If we ever do that later, I think we'd need a safety net like this one. |
||
RepositoryDescription existingDescription = requireNonNull(fetchRepositoryDescription()); | ||
RepositoryDescription sanitizedDescription = | ||
ImmutableRepositoryDescription.builder() | ||
.from(newDescription) | ||
.oldestPossibleCommitTime(existingDescription.oldestPossibleCommitTime()) | ||
.repositoryCreatedTime(existingDescription.repositoryCreatedTime()) | ||
.defaultBranchName(existingDescription.defaultBranchName()) | ||
.build(); | ||
byte[] serialized = serialize(sanitizedDescription); | ||
try { | ||
StringValue existing = | ||
commitRetry( | ||
persist, | ||
(p, retryState) -> { | ||
try { | ||
Reference reference = requireNonNull(persist.fetchReference(REF_REPO.name())); | ||
return stringLogic(persist) | ||
.updateStringOnRef( | ||
reference, | ||
KEY_REPO_DESCRIPTION, | ||
b -> | ||
b.message("Update repository description") | ||
.commitType(CommitType.INTERNAL), | ||
"application/json", | ||
serialized); | ||
} catch (RefConditionFailedException | CommitConflictException e) { | ||
throw new RetryException(); | ||
} catch (ObjNotFoundException | RefNotFoundException e) { | ||
throw new CommitWrappedException(e); | ||
} | ||
}); | ||
return existing != null ? deserialize(existing) : null; | ||
} catch (CommitConflictException e) { | ||
throw new RuntimeException( | ||
"An unexpected internal error happened while committing a repository description update"); | ||
} catch (CommitWrappedException e) { | ||
throw new RuntimeException( | ||
"An unexpected internal error happened while committing a repository description update", | ||
e.getCause()); | ||
} | ||
} | ||
|
||
@SuppressWarnings({"JavaTimeDefaultTimeZone"}) | ||
private void initializeInternalRef( | ||
InternalRef internalRef, Consumer<CreateCommit.Builder> commitEnhancer) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh, exactly this one might be updated. Let's remove "defaultBranchName" from that check.
I'm not sure whether there'll ever be a command to update arbitrary information - rather per attribute.