-
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 4 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 |
---|---|---|
|
@@ -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; | ||
|
@@ -33,17 +34,23 @@ | |
|
||
import java.io.IOException; | ||
import java.time.Instant; | ||
import java.util.Objects; | ||
import java.util.Optional; | ||
import java.util.UUID; | ||
import java.util.function.Consumer; | ||
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 +142,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 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 +184,51 @@ 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)); | ||
b.addAdds( | ||
commitAdd(KEY_REPO_DESCRIPTION, 0, requireNonNull(string.id()), null, UUID.randomUUID())); | ||
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 change will only work for new repositories, but existing repositories do not have a content-ID for the repo-description. The update-logic does not work without a content-ID. I'd prefer to leave the content-ID as I know that the repo-config stuff uses a CID, see comment in SLImpl |
||
} catch (ObjTooLargeException | ObjNotFoundException | IOException e) { | ||
throw new RuntimeException(e); | ||
} | ||
} | ||
|
||
@Override | ||
public RepositoryDescription updateRepositoryDescription( | ||
RepositoryDescription repositoryDescription) throws RetryTimeoutException { | ||
try { | ||
return commitRetry( | ||
persist, | ||
(p, retryState) -> { | ||
try { | ||
Reference reference = Objects.requireNonNull(persist.fetchReference(REF_REPO.name())); | ||
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. We should probably fix the javadoc of |
||
StringValue existing = | ||
stringLogic(persist) | ||
.updateStringOnRef( | ||
reference, | ||
KEY_REPO_DESCRIPTION, | ||
b -> | ||
b.message("Update repository description") | ||
.commitType(CommitType.INTERNAL), | ||
"application/json", | ||
serialize(repositoryDescription)); | ||
return existing != null ? deserialize(existing) : null; | ||
} catch (RefConditionFailedException | CommitConflictException e) { | ||
throw new RetryException(); | ||
} catch (ObjNotFoundException | RefNotFoundException e) { | ||
throw new CommitWrappedException(e); | ||
} | ||
}); | ||
} 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) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,17 +17,35 @@ | |
|
||
import static com.google.common.base.Preconditions.checkState; | ||
import static java.util.Collections.emptyList; | ||
import static java.util.Collections.singletonList; | ||
import static java.util.Objects.requireNonNull; | ||
import static org.projectnessie.nessie.relocated.protobuf.UnsafeByteOperations.unsafeWrap; | ||
import static org.projectnessie.versioned.storage.common.logic.CreateCommit.Add.commitAdd; | ||
import static org.projectnessie.versioned.storage.common.logic.Logics.commitLogic; | ||
import static org.projectnessie.versioned.storage.common.logic.Logics.indexesLogic; | ||
import static org.projectnessie.versioned.storage.common.objtypes.CommitHeaders.EMPTY_COMMIT_HEADERS; | ||
import static org.projectnessie.versioned.storage.common.objtypes.StringObj.stringData; | ||
import static org.projectnessie.versioned.storage.common.persist.ObjType.STRING; | ||
|
||
import java.nio.charset.StandardCharsets; | ||
import java.util.UUID; | ||
import java.util.function.Consumer; | ||
import org.projectnessie.nessie.relocated.protobuf.ByteString; | ||
import org.projectnessie.versioned.storage.common.exceptions.CommitConflictException; | ||
import org.projectnessie.versioned.storage.common.exceptions.ObjNotFoundException; | ||
import org.projectnessie.versioned.storage.common.exceptions.RefConditionFailedException; | ||
import org.projectnessie.versioned.storage.common.exceptions.RefNotFoundException; | ||
import org.projectnessie.versioned.storage.common.indexes.StoreIndex; | ||
import org.projectnessie.versioned.storage.common.indexes.StoreIndexElement; | ||
import org.projectnessie.versioned.storage.common.indexes.StoreKey; | ||
import org.projectnessie.versioned.storage.common.logic.CreateCommit.Builder; | ||
import org.projectnessie.versioned.storage.common.objtypes.CommitObj; | ||
import org.projectnessie.versioned.storage.common.objtypes.CommitOp; | ||
import org.projectnessie.versioned.storage.common.objtypes.Compression; | ||
import org.projectnessie.versioned.storage.common.objtypes.StringObj; | ||
import org.projectnessie.versioned.storage.common.persist.ObjId; | ||
import org.projectnessie.versioned.storage.common.persist.Persist; | ||
import org.projectnessie.versioned.storage.common.persist.Reference; | ||
|
||
final class StringLogicImpl implements StringLogic { | ||
private final Persist persist; | ||
|
@@ -68,6 +86,53 @@ public StringObj updateString(StringValue previousValue, String contentType, Str | |
return updateString(previousValue, contentType, stringValue.getBytes(StandardCharsets.UTF_8)); | ||
} | ||
|
||
@Override | ||
public StringValue updateStringOnRef( | ||
Reference reference, | ||
StoreKey storeKey, | ||
Consumer<Builder> commitEnhancer, | ||
String contentType, | ||
byte[] stringValueUtf8) | ||
throws ObjNotFoundException, | ||
CommitConflictException, | ||
RefNotFoundException, | ||
RefConditionFailedException { | ||
CommitLogic commitLogic = commitLogic(persist); | ||
IndexesLogic indexesLogic = indexesLogic(persist); | ||
CommitObj head = commitLogic.headCommit(reference); | ||
StoreIndex<CommitOp> index = indexesLogic.buildCompleteIndexOrEmpty(head); | ||
StoreIndexElement<CommitOp> existingElement = index.get(storeKey); | ||
ObjId existingValueId = null; | ||
UUID existingContentId = null; | ||
StringValue existing = null; | ||
if (existingElement != null) { | ||
CommitOp op = existingElement.content(); | ||
if (op.action().exists()) { | ||
existingValueId = op.value(); | ||
existingContentId = op.contentId(); | ||
existing = existingValueId != null ? fetchString(requireNonNull(existingValueId)) : null; | ||
} | ||
} | ||
if (existingContentId == null) { | ||
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 makes updating repo-descriptions of already existing repos impossible (those have no CID for repo-description). Mabe replacing the Should also have some comment to explain the logic wrt repo-desc + repo-config |
||
existingContentId = UUID.randomUUID(); | ||
} | ||
|
||
StringObj newValue = updateString(existing, contentType, stringValueUtf8); | ||
|
||
ObjId newValueId = newValue.id(); | ||
if (!requireNonNull(newValueId).equals(existingValueId)) { | ||
CreateCommit.Builder builder = | ||
CreateCommit.newCommitBuilder() | ||
.parentCommitId(reference.pointer()) | ||
.headers(EMPTY_COMMIT_HEADERS) | ||
.addAdds(commitAdd(storeKey, 0, newValueId, existingValueId, existingContentId)); | ||
commitEnhancer.accept(builder); | ||
CommitObj committed = commitLogic.doCommit(builder.build(), singletonList(newValue)); | ||
persist.updateReferencePointer(reference, requireNonNull(committed).id()); | ||
} | ||
return existing; | ||
} | ||
|
||
static final class StringValueHolder implements StringValue { | ||
final StringObj obj; | ||
|
||
|
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.
This and the below setting are actually read-only attributes.