diff --git a/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java b/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java index f9bdb2fb9f..091b02c22e 100644 --- a/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java +++ b/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java @@ -91,6 +91,8 @@ public class FileDiffCacheImpl implements FileDiffCache { persist(DIFF, FileDiffCacheKey.class, FileDiffOutput.class) .maximumWeight(10 << 20) .weigher(FileDiffWeigher.class) + .keySerializer(FileDiffCacheKey.Serializer.INSTANCE) + .valueSerializer(FileDiffOutput.Serializer.INSTANCE) .loader(FileDiffLoader.class); } }; diff --git a/java/com/google/gerrit/server/patch/filediff/FileDiffCacheKey.java b/java/com/google/gerrit/server/patch/filediff/FileDiffCacheKey.java index 5e45da2999..a478fcf6aa 100644 --- a/java/com/google/gerrit/server/patch/filediff/FileDiffCacheKey.java +++ b/java/com/google/gerrit/server/patch/filediff/FileDiffCacheKey.java @@ -21,6 +21,10 @@ import com.google.gerrit.entities.Project; import com.google.gerrit.entities.Project.NameKey; import com.google.gerrit.extensions.client.DiffPreferencesInfo; import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace; +import com.google.gerrit.proto.Protos; +import com.google.gerrit.server.cache.proto.Cache.FileDiffKeyProto; +import com.google.gerrit.server.cache.serialize.CacheSerializer; +import com.google.gerrit.server.cache.serialize.ObjectIdConverter; import com.google.gerrit.server.patch.gitfilediff.GitFileDiffCacheImpl.DiffAlgorithm; import org.eclipse.jgit.lib.ObjectId; @@ -89,4 +93,38 @@ public abstract class FileDiffCacheKey { public abstract FileDiffCacheKey build(); } + + public enum Serializer implements CacheSerializer { + INSTANCE; + + @Override + public byte[] serialize(FileDiffCacheKey key) { + ObjectIdConverter idConverter = ObjectIdConverter.create(); + return Protos.toByteArray( + FileDiffKeyProto.newBuilder() + .setProject(key.project().get()) + .setOldCommit(idConverter.toByteString(key.oldCommit())) + .setNewCommit(idConverter.toByteString(key.newCommit())) + .setFilePath(key.newFilePath()) + .setRenameScore(key.renameScore()) + .setDiffAlgorithm(key.diffAlgorithm().name()) + .setWhitespace(key.whitespace().name()) + .build()); + } + + @Override + public FileDiffCacheKey deserialize(byte[] in) { + FileDiffKeyProto proto = Protos.parseUnchecked(FileDiffKeyProto.parser(), in); + ObjectIdConverter idConverter = ObjectIdConverter.create(); + return FileDiffCacheKey.builder() + .project(Project.nameKey(proto.getProject())) + .oldCommit(idConverter.fromByteString(proto.getOldCommit())) + .newCommit(idConverter.fromByteString(proto.getNewCommit())) + .newFilePath(proto.getFilePath()) + .renameScore(proto.getRenameScore()) + .diffAlgorithm(DiffAlgorithm.valueOf(proto.getDiffAlgorithm())) + .whitespace(Whitespace.valueOf(proto.getWhitespace())) + .build(); + } + } } diff --git a/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java b/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java index c161cfa0e3..e89f1481a7 100644 --- a/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java +++ b/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java @@ -21,8 +21,13 @@ import com.google.common.collect.ImmutableList; import com.google.gerrit.entities.Patch; import com.google.gerrit.entities.Patch.ChangeType; import com.google.gerrit.entities.Patch.PatchType; +import com.google.gerrit.proto.Protos; +import com.google.gerrit.server.cache.proto.Cache.FileDiffOutputProto; +import com.google.gerrit.server.cache.serialize.CacheSerializer; +import com.google.protobuf.Descriptors.FieldDescriptor; import java.io.Serializable; import java.util.Optional; +import java.util.stream.Collectors; /** File diff for a single file path. Produced as output of the {@link FileDiffCache}. */ @AutoValue @@ -105,7 +110,7 @@ public abstract class FileDiffOutput implements Serializable { return headerLines().isEmpty() && edits().isEmpty(); } - static Builder builder() { + public static Builder builder() { return new AutoValue_FileDiffOutput.Builder(); } @@ -153,4 +158,98 @@ public abstract class FileDiffOutput implements Serializable { public abstract FileDiffOutput build(); } + + public enum Serializer implements CacheSerializer { + INSTANCE; + + private static final FieldDescriptor OLD_PATH_DESCRIPTOR = + FileDiffOutputProto.getDescriptor().findFieldByNumber(1); + + private static final FieldDescriptor NEW_PATH_DESCRIPTOR = + FileDiffOutputProto.getDescriptor().findFieldByNumber(2); + + private static final FieldDescriptor CHANGE_TYPE_DESCRIPTOR = + FileDiffOutputProto.getDescriptor().findFieldByNumber(3); + + private static final FieldDescriptor PATCH_TYPE_DESCRIPTOR = + FileDiffOutputProto.getDescriptor().findFieldByNumber(4); + + @Override + public byte[] serialize(FileDiffOutput fileDiff) { + FileDiffOutputProto.Builder builder = + FileDiffOutputProto.newBuilder() + .setSize(fileDiff.size()) + .setSizeDelta(fileDiff.sizeDelta()) + .addAllHeaderLines(fileDiff.headerLines()) + .addAllEdits( + fileDiff.edits().stream() + .map( + e -> + FileDiffOutputProto.TaggedEdit.newBuilder() + .setEdit( + FileDiffOutputProto.Edit.newBuilder() + .setBeginA(e.edit().beginA()) + .setEndA(e.edit().endA()) + .setBeginB(e.edit().beginB()) + .setEndB(e.edit().endB()) + .build()) + .setDueToRebase(e.dueToRebase()) + .build()) + .collect(Collectors.toList())); + + if (fileDiff.oldPath().isPresent()) { + builder.setOldPath(fileDiff.oldPath().get()); + } + + if (fileDiff.newPath().isPresent()) { + builder.setNewPath(fileDiff.newPath().get()); + } + + if (fileDiff.changeType().isPresent()) { + builder.setChangeType(fileDiff.changeType().get().name()); + } + + if (fileDiff.patchType().isPresent()) { + builder.setPatchType(fileDiff.patchType().get().name()); + } + + return Protos.toByteArray(builder.build()); + } + + @Override + public FileDiffOutput deserialize(byte[] in) { + FileDiffOutputProto proto = Protos.parseUnchecked(FileDiffOutputProto.parser(), in); + FileDiffOutput.Builder builder = FileDiffOutput.builder(); + builder + .size(proto.getSize()) + .sizeDelta(proto.getSizeDelta()) + .headerLines(proto.getHeaderLinesList().stream().collect(ImmutableList.toImmutableList())) + .edits( + proto.getEditsList().stream() + .map( + e -> + TaggedEdit.create( + Edit.create( + e.getEdit().getBeginA(), + e.getEdit().getEndA(), + e.getEdit().getBeginB(), + e.getEdit().getEndB()), + e.getDueToRebase())) + .collect(ImmutableList.toImmutableList())); + + if (proto.hasField(OLD_PATH_DESCRIPTOR)) { + builder.oldPath(Optional.of(proto.getOldPath())); + } + if (proto.hasField(NEW_PATH_DESCRIPTOR)) { + builder.newPath(Optional.of(proto.getNewPath())); + } + if (proto.hasField(CHANGE_TYPE_DESCRIPTOR)) { + builder.changeType(Optional.of(Patch.ChangeType.valueOf(proto.getChangeType()))); + } + if (proto.hasField(PATCH_TYPE_DESCRIPTOR)) { + builder.patchType(Optional.of(Patch.PatchType.valueOf(proto.getPatchType()))); + } + return builder.build(); + } + } } diff --git a/java/com/google/gerrit/server/patch/gitdiff/ModifiedFile.java b/java/com/google/gerrit/server/patch/gitdiff/ModifiedFile.java index 800bd417f3..9512094cb3 100644 --- a/java/com/google/gerrit/server/patch/gitdiff/ModifiedFile.java +++ b/java/com/google/gerrit/server/patch/gitdiff/ModifiedFile.java @@ -79,10 +79,10 @@ public abstract class ModifiedFile { INSTANCE; private static final FieldDescriptor oldPathDescriptor = - ModifiedFileProto.getDescriptor().findFieldByName("old_path"); + ModifiedFileProto.getDescriptor().findFieldByNumber(2); private static final FieldDescriptor newPathDescriptor = - ModifiedFileProto.getDescriptor().findFieldByName("new_path"); + ModifiedFileProto.getDescriptor().findFieldByNumber(3); @Override public byte[] serialize(ModifiedFile modifiedFile) { diff --git a/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiff.java b/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiff.java index 7d1c24ed6e..6d63243119 100644 --- a/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiff.java +++ b/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiff.java @@ -202,22 +202,22 @@ public abstract class GitFileDiff { INSTANCE; private static final FieldDescriptor OLD_PATH_DESCRIPTOR = - GitFileDiffProto.getDescriptor().findFieldByName("old_path"); + GitFileDiffProto.getDescriptor().findFieldByNumber(3); private static final FieldDescriptor NEW_PATH_DESCRIPTOR = - GitFileDiffProto.getDescriptor().findFieldByName("new_path"); + GitFileDiffProto.getDescriptor().findFieldByNumber(4); private static final FieldDescriptor OLD_MODE_DESCRIPTOR = - GitFileDiffProto.getDescriptor().findFieldByName("old_mode"); + GitFileDiffProto.getDescriptor().findFieldByNumber(7); private static final FieldDescriptor NEW_MODE_DESCRIPTOR = - GitFileDiffProto.getDescriptor().findFieldByName("new_mode"); + GitFileDiffProto.getDescriptor().findFieldByNumber(8); private static final FieldDescriptor CHANGE_TYPE_DESCRIPTOR = - GitFileDiffProto.getDescriptor().findFieldByName("change_type"); + GitFileDiffProto.getDescriptor().findFieldByNumber(9); private static final FieldDescriptor PATCH_TYPE_DESCRIPTOR = - GitFileDiffProto.getDescriptor().findFieldByName("patch_type"); + GitFileDiffProto.getDescriptor().findFieldByNumber(10); @Override public byte[] serialize(GitFileDiff gitFileDiff) { diff --git a/javatests/com/google/gerrit/server/cache/serialize/entities/FileDiffCacheKeySerializerTest.java b/javatests/com/google/gerrit/server/cache/serialize/entities/FileDiffCacheKeySerializerTest.java new file mode 100644 index 0000000000..ecab07d476 --- /dev/null +++ b/javatests/com/google/gerrit/server/cache/serialize/entities/FileDiffCacheKeySerializerTest.java @@ -0,0 +1,50 @@ +// Copyright (C) 2020 The Android Open Source Project +// +// 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.google.gerrit.server.cache.serialize.entities; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.gerrit.entities.Project; +import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace; +import com.google.gerrit.server.patch.filediff.FileDiffCacheKey; +import com.google.gerrit.server.patch.gitfilediff.GitFileDiffCacheImpl.DiffAlgorithm; +import org.eclipse.jgit.lib.ObjectId; +import org.junit.Test; + +/** Tests the protobuf serializer for the {@link FileDiffCacheKey}. */ +public class FileDiffCacheKeySerializerTest { + private static final ObjectId COMMIT_ID_1 = + ObjectId.fromString("123e9fa8a286255ac7d5ba11b598892735758391"); + private static final ObjectId COMMIT_ID_2 = + ObjectId.fromString("d07a03a9818c120301cb5b4a969b035479400b5f"); + + @Test + public void roundTrip() { + FileDiffCacheKey key = + FileDiffCacheKey.builder() + .project(Project.nameKey("project/x")) + .oldCommit(COMMIT_ID_1) + .newCommit(COMMIT_ID_2) + .newFilePath("some_file.txt") + .renameScore(65) + .diffAlgorithm(DiffAlgorithm.HISTOGRAM) + .whitespace(Whitespace.IGNORE_ALL) + .build(); + + byte[] serialized = FileDiffCacheKey.Serializer.INSTANCE.serialize(key); + + assertThat(FileDiffCacheKey.Serializer.INSTANCE.deserialize(serialized)).isEqualTo(key); + } +} diff --git a/javatests/com/google/gerrit/server/cache/serialize/entities/FileDiffOutputSerializerTest.java b/javatests/com/google/gerrit/server/cache/serialize/entities/FileDiffOutputSerializerTest.java new file mode 100644 index 0000000000..43079545a8 --- /dev/null +++ b/javatests/com/google/gerrit/server/cache/serialize/entities/FileDiffOutputSerializerTest.java @@ -0,0 +1,51 @@ +// Copyright (C) 2020 The Android Open Source Project +// +// 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.google.gerrit.server.cache.serialize.entities; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableList; +import com.google.gerrit.entities.Patch.ChangeType; +import com.google.gerrit.entities.Patch.PatchType; +import com.google.gerrit.server.patch.filediff.Edit; +import com.google.gerrit.server.patch.filediff.FileDiffOutput; +import com.google.gerrit.server.patch.filediff.TaggedEdit; +import java.util.Optional; +import org.junit.Test; + +public class FileDiffOutputSerializerTest { + @Test + public void roundTrip() { + ImmutableList edits = + ImmutableList.of( + TaggedEdit.create(Edit.create(1, 5, 3, 4), true), + TaggedEdit.create(Edit.create(21, 30, 150, 158), false)); + + FileDiffOutput fileDiff = + FileDiffOutput.builder() + .oldPath(Optional.of("old_file_path.txt")) + .newPath(Optional.empty()) + .changeType(Optional.of(ChangeType.DELETED)) + .patchType(Optional.of(PatchType.UNIFIED)) + .size(23) + .sizeDelta(10) + .headerLines(ImmutableList.of("header line 1", "header line 2")) + .edits(edits) + .build(); + + byte[] serialized = FileDiffOutput.Serializer.INSTANCE.serialize(fileDiff); + assertThat(FileDiffOutput.Serializer.INSTANCE.deserialize(serialized)).isEqualTo(fileDiff); + } +} diff --git a/proto/cache.proto b/proto/cache.proto index 0676f1b59a..8d575b7c70 100644 --- a/proto/cache.proto +++ b/proto/cache.proto @@ -599,3 +599,43 @@ message GitFileDiffProto { string change_type = 9; // ENUM as string string patch_type = 10; // ENUM as string } + +// Serialized form of +// com.google.gerrit.server.patch.fileDiff.FileDiffCacheKey +// Next ID: 8 +message FileDiffKeyProto { + string project = 1; + bytes old_commit = 2; + bytes new_commit = 3; + string file_path = 4; + int32 rename_score = 5; + string diff_algorithm = 6; + string whitespace = 7; +} + +// Serialized form of +// com.google.gerrit.server.patch.filediff.FileDiffOutput +// Next ID: 9 +message FileDiffOutputProto { + // Next ID: 5 + message Edit { + int32 begin_a = 1; + int32 end_a = 2; + int32 begin_b = 3; + int32 end_b = 4; + } + // Serialized form of com.google.gerrit.server.patch.filediff.TaggedEdit + // Next ID: 3 + message TaggedEdit { + Edit edit = 1; + bool due_to_rebase = 2; + } + string old_path = 1; + string new_path = 2; + string change_type = 3; + string patch_type = 4; + repeated string header_lines = 5; + int64 size = 6; + int64 size_delta = 7; + repeated TaggedEdit edits = 8; +}