Support ApplyObject of non-commit refs Enable to apply non-commit refs directly through the ApplyObject action, instead of requiring the execution of a git fetch. Rationale: the git fetch is not optimised for replicating non-commits, because it relies on the reachability graph of commit objects. The transmission of a ref-BLOB pair is straightforward and implies the simple insertion of a byte[] with an object type associated with a ref. Change-Id: I90b74a60f9077bbbad29c38c5035ea12ee216c71
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/RevisionReader.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/RevisionReader.java index 468c5fc..92cbd95 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/RevisionReader.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/RevisionReader.java
@@ -17,6 +17,7 @@ import static com.googlesource.gerrit.plugins.replication.pull.PullReplicationLogger.repLog; import com.google.common.collect.Lists; +import com.google.gerrit.common.Nullable; import com.google.gerrit.entities.Project; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.inject.Inject; @@ -24,6 +25,7 @@ import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionData; import com.googlesource.gerrit.plugins.replication.pull.api.data.RevisionObjectData; import java.io.IOException; +import java.util.Arrays; import java.util.List; import java.util.Optional; import org.eclipse.jgit.diff.DiffEntry; @@ -36,6 +38,7 @@ import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectLoader; +import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevTree; @@ -56,16 +59,39 @@ .getLong("replication", CONFIG_MAX_API_PAYLOAD_SIZE, DEFAULT_MAX_PAYLOAD_SIZE_IN_BYTES); } - public Optional<RevisionData> read(Project.NameKey project, ObjectId objectId, String refName) + public Optional<RevisionData> read(Project.NameKey project, String refName) + throws RepositoryNotFoundException, MissingObjectException, IncorrectObjectTypeException, + CorruptObjectException, IOException { + return read(project, null, refName); + } + + public Optional<RevisionData> read( + Project.NameKey project, @Nullable ObjectId refObjectId, String refName) throws MissingObjectException, IncorrectObjectTypeException, CorruptObjectException, RepositoryNotFoundException, IOException { try (Repository git = gitRepositoryManager.openRepository(project)) { Long totalRefSize = 0l; + Ref ref = git.exactRef(refName); + if (ref == null) { + return Optional.empty(); + } + + ObjectId objectId = refObjectId == null ? ref.getObjectId() : refObjectId; + ObjectLoader commitLoader = git.open(objectId); totalRefSize += commitLoader.getSize(); verifySize(totalRefSize, commitLoader); + if (commitLoader.getType() == Constants.OBJ_BLOB) { + return Optional.of( + new RevisionData( + null, + null, + Arrays.asList( + new RevisionObjectData(Constants.OBJ_BLOB, commitLoader.getCachedBytes())))); + } + if (commitLoader.getType() != Constants.OBJ_COMMIT) { repLog.trace( "Ref {} for project {} points to an object type {}",
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectAction.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectAction.java index 5132f41..f83cadb 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectAction.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectAction.java
@@ -65,17 +65,10 @@ return Response.created(input); } - if (Objects.isNull(input.getRevisionData().getCommitObject()) - || Objects.isNull(input.getRevisionData().getCommitObject().getContent()) - || input.getRevisionData().getCommitObject().getContent().length == 0 - || Objects.isNull(input.getRevisionData().getCommitObject().getType())) { - throw new BadRequestException("Ref-update commit object cannot be null or empty"); - } - - if (Objects.isNull(input.getRevisionData().getTreeObject()) - || Objects.isNull(input.getRevisionData().getTreeObject().getContent()) - || Objects.isNull(input.getRevisionData().getTreeObject().getType())) { - throw new BadRequestException("Ref-update tree object cannot be null"); + try { + input.validate(); + } catch (IllegalArgumentException e) { + throw new BadRequestException("Ref-update with invalid input: " + e.getMessage(), e); } command.applyObject(
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/data/RevisionInput.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/data/RevisionInput.java index bc3e218..18fc27e 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/data/RevisionInput.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/data/RevisionInput.java
@@ -14,6 +14,10 @@ package com.googlesource.gerrit.plugins.replication.pull.api.data; +import java.util.List; +import java.util.Objects; +import org.eclipse.jgit.lib.Constants; + public class RevisionInput { private String label; @@ -38,4 +42,40 @@ public RevisionData getRevisionData() { return revisionData; } + + public void validate() { + // Non-heads refs can point to non-commit objects + if (!refName.startsWith(Constants.R_HEADS) + && Objects.isNull(revisionData.getCommitObject()) + && Objects.isNull(revisionData.getTreeObject())) { + + List<RevisionObjectData> blobs = revisionData.getBlobs(); + + if (Objects.isNull(blobs) || blobs.isEmpty()) { + throw new IllegalArgumentException( + "Ref " + refName + " cannot have a null or empty list of BLOBs associated"); + } + + if (blobs.size() > 1) { + throw new IllegalArgumentException("Ref " + refName + " has more than one BLOB associated"); + } + + return; + } + + if (Objects.isNull(revisionData.getCommitObject()) + || Objects.isNull(revisionData.getCommitObject().getContent()) + || revisionData.getCommitObject().getContent().length == 0 + || Objects.isNull(revisionData.getCommitObject().getType())) { + throw new IllegalArgumentException( + "Commit object for ref " + refName + " cannot be null or empty"); + } + + if (Objects.isNull(revisionData.getTreeObject()) + || Objects.isNull(revisionData.getTreeObject().getContent()) + || Objects.isNull(revisionData.getTreeObject().getType())) { + throw new IllegalArgumentException( + "Ref-update tree object for ref " + refName + " cannot be null"); + } + } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/ApplyObject.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/ApplyObject.java index 03362bf..fca085b 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/ApplyObject.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/fetch/ApplyObject.java
@@ -46,27 +46,39 @@ try (Repository git = gitManager.openRepository(name)) { ObjectId newObjectID = null; - try (ObjectInserter oi = git.newObjectInserter()) { - RevisionObjectData commitObject = revisionData.getCommitObject(); - RevCommit commit = RevCommit.parse(commitObject.getContent()); - for (RevCommit parent : commit.getParents()) { - if (!git.getObjectDatabase().has(parent.getId())) { - throw new MissingParentObjectException(name, refSpec.getSource(), parent.getId()); - } - } - newObjectID = oi.insert(commitObject.getType(), commitObject.getContent()); + RevisionObjectData commitObject = revisionData.getCommitObject(); - RevisionObjectData treeObject = revisionData.getTreeObject(); - oi.insert(treeObject.getType(), treeObject.getContent()); + try (ObjectInserter oi = git.newObjectInserter()) { + if (commitObject != null) { + RevCommit commit = RevCommit.parse(commitObject.getContent()); + for (RevCommit parent : commit.getParents()) { + if (!git.getObjectDatabase().has(parent.getId())) { + throw new MissingParentObjectException(name, refSpec.getSource(), parent.getId()); + } + } + newObjectID = oi.insert(commitObject.getType(), commitObject.getContent()); + + RevisionObjectData treeObject = revisionData.getTreeObject(); + oi.insert(treeObject.getType(), treeObject.getContent()); + } for (RevisionObjectData rev : revisionData.getBlobs()) { - oi.insert(rev.getType(), rev.getContent()); + ObjectId blobObjectId = oi.insert(rev.getType(), rev.getContent()); + if (newObjectID == null) { + newObjectID = blobObjectId; + } } oi.flush(); } RefUpdate ru = git.updateRef(refSpec.getSource()); ru.setNewObjectId(newObjectID); + + if (commitObject == null) { + // Non-commits must be forced as they do not have a graph associated + ru.setForceUpdate(true); + } + RefUpdate.Result result = ru.update(); return new RefUpdateState(refSpec.getSource(), result);
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectActionTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectActionTest.java index 56e1429..3678e82 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectActionTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ApplyObjectActionTest.java
@@ -35,6 +35,8 @@ import com.googlesource.gerrit.plugins.replication.pull.api.exception.MissingParentObjectException; import com.googlesource.gerrit.plugins.replication.pull.api.exception.RefUpdateException; import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.junit.Before; @@ -49,6 +51,7 @@ String label = "instance-2-label"; String url = "file:///gerrit-host/instance-1/git/${name}.git"; String refName = "refs/heads/master"; + String refMetaName = "refs/meta/version"; String location = "http://gerrit-host/a/config/server/tasks/08d173e9"; int taskId = 1234; @@ -92,6 +95,20 @@ assertThat(response.statusCode()).isEqualTo(SC_CREATED); } + @Test + public void shouldReturnCreatedResponseCodeForBlob() throws RestApiException { + byte[] blobData = "foo".getBytes(StandardCharsets.UTF_8); + RevisionInput inputParams = + new RevisionInput( + label, + refMetaName, + createSampleRevisionDataBlob(new RevisionObjectData(Constants.OBJ_BLOB, blobData))); + + Response<?> response = applyObjectAction.apply(projectResource, inputParams); + + assertThat(response.statusCode()).isEqualTo(SC_CREATED); + } + @SuppressWarnings("cast") @Test public void shouldReturnSourceUrlAndrefNameAsAResponseBody() throws Exception { @@ -184,4 +201,8 @@ RevisionObjectData commitData, RevisionObjectData treeData) { return new RevisionData(commitData, treeData, Lists.newArrayList()); } + + private RevisionData createSampleRevisionDataBlob(RevisionObjectData blob) { + return new RevisionData(null, null, Arrays.asList(blob)); + } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/fetch/ApplyObjectIT.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/fetch/ApplyObjectIT.java index e2a162e..b24d1d7 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/fetch/ApplyObjectIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/fetch/ApplyObjectIT.java
@@ -93,6 +93,28 @@ } @Test + public void shouldApplyRefSequencesChanges() throws Exception { + String testRepoProjectName = project + TEST_REPLICATION_SUFFIX; + testRepo = cloneProject(createTestProject(testRepoProjectName)); + + createChange(); + String seqChangesRef = RefNames.REFS_SEQUENCES + "changes"; + + Optional<RevisionData> revisionData = reader.read(allProjects, seqChangesRef); + + RefSpec refSpec = new RefSpec(seqChangesRef); + objectUnderTest.apply(project, refSpec, revisionData.get()); + try (Repository repo = repoManager.openRepository(project); + TestRepository<Repository> testRepo = new TestRepository<>(repo); ) { + + Optional<RevisionData> newRevisionData = + reader.read(project, repo.exactRef(seqChangesRef).getObjectId(), seqChangesRef); + compareObjects(revisionData.get(), newRevisionData); + testRepo.fsck(); + } + } + + @Test public void shouldApplyRefMetaObjectWithComments() throws Exception { String testRepoProjectName = project + TEST_REPLICATION_SUFFIX; testRepo = cloneProject(createTestProject(testRepoProjectName)); @@ -173,6 +195,9 @@ } private void compareContent(RevisionObjectData expected, RevisionObjectData actual) { + if (expected == actual) { + return; + } assertThat(actual.getType()).isEqualTo(expected.getType()); assertThat(Bytes.asList(actual.getContent())) .containsExactlyElementsIn(Bytes.asList(expected.getContent()))