From 04d7932eb3942ab0a6d8e0641de1279be5359c49 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Fri, 31 Jan 2014 10:14:00 -0800 Subject: [PATCH] Add tests for LabelNormalizer These use the full InMemoryModule stack because depending on users, groups, label types, and permissions is heavyweight. The careful reader will notice based on the tested behavior that there are still some outstanding bugs in label normalization during submit time. For example, since Submit only upserts the normalized list of approvals, any labels omitted by the normalizer due to an empty permission range are left entirely untouched. Such bugs are not fixed by this change, which is only to add tests. Change-Id: I955ee8c63b17812684f8350113cb0f2f1bf72fe6 --- .../reviewdb/client/PatchSetApproval.java | 17 ++ .../gerrit/server/git/LabelNormalizer.java | 6 +- .../server/git/LabelNormalizerTest.java | 207 ++++++++++++++++++ 3 files changed, 227 insertions(+), 3 deletions(-) create mode 100644 gerrit-server/src/test/java/com/google/gerrit/server/git/LabelNormalizerTest.java diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java index b97865d245..83a9b67d20 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java @@ -19,6 +19,7 @@ import com.google.gwtorm.client.CompoundKey; import com.google.gwtorm.client.StringKey; import java.sql.Timestamp; +import java.util.Objects; /** An approval (or negative approval) on a patch set. */ public final class PatchSetApproval { @@ -190,4 +191,20 @@ public final class PatchSetApproval { return new StringBuilder().append('[').append(key).append(": ") .append(value).append(']').toString(); } + + @Override + public boolean equals(Object o) { + if (o instanceof PatchSetApproval) { + PatchSetApproval p = (PatchSetApproval) o; + return Objects.equals(key, p.key) + && Objects.equals(value, p.value) + && Objects.equals(granted, p.granted); + } + return false; + } + + @Override + public int hashCode() { + return Objects.hash(key, value, granted); + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java index 03529567b0..499b8d783a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java @@ -88,12 +88,12 @@ public class LabelNormalizer { "Approval %s does not match change %s", psa.getKey(), ctl.getChange().getKey()); if (psa.isSubmit()) { - result.add(copy(psa, ctl)); + result.add(copy(psa)); continue; } LabelType label = labelTypes.byLabel(psa.getLabelId()); if (label != null) { - psa = copy(psa, ctl); + psa = copy(psa); applyTypeFloor(label, psa); if (applyRightFloor(ctl, label, psa)) { result.add(psa); @@ -114,7 +114,7 @@ public class LabelNormalizer { return !getRange(ctl, lt, id).isEmpty(); } - private PatchSetApproval copy(PatchSetApproval src, ChangeControl ctl) { + private PatchSetApproval copy(PatchSetApproval src) { return new PatchSetApproval(src.getPatchSetId(), src); } diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/git/LabelNormalizerTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/git/LabelNormalizerTest.java new file mode 100644 index 0000000000..cae5329a79 --- /dev/null +++ b/gerrit-server/src/test/java/com/google/gerrit/server/git/LabelNormalizerTest.java @@ -0,0 +1,207 @@ +// Copyright (C) 2014 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.git; + +import static com.google.gerrit.common.data.Permission.forLabel; +import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; +import static com.google.gerrit.server.project.Util.category; +import static com.google.gerrit.server.project.Util.grant; +import static com.google.gerrit.server.project.Util.value; +import static org.junit.Assert.assertEquals; + +import com.google.common.collect.ImmutableList; +import com.google.gerrit.common.data.AccessSection; +import com.google.gerrit.common.data.LabelType; +import com.google.gerrit.lifecycle.LifecycleManager; +import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.reviewdb.client.Branch; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.PatchSet; +import com.google.gerrit.reviewdb.client.PatchSetApproval; +import com.google.gerrit.reviewdb.client.PatchSetApproval.LabelId; +import com.google.gerrit.reviewdb.client.PatchSetInfo; +import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.ChangeUtil; +import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.account.AccountManager; +import com.google.gerrit.server.account.AuthRequest; +import com.google.gerrit.server.config.AllProjectsName; +import com.google.gerrit.server.project.ProjectCache; +import com.google.gerrit.server.schema.SchemaCreator; +import com.google.gerrit.server.util.TimeUtil; +import com.google.gerrit.testutil.InMemoryDatabase; +import com.google.gerrit.testutil.InMemoryModule; +import com.google.inject.Guice; +import com.google.inject.Inject; +import com.google.inject.Injector; + +import org.eclipse.jgit.lib.Repository; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +/** Unit tests for {@link LabelNormalizer}. */ +public class LabelNormalizerTest { + @Inject private AccountManager accountManager; + @Inject private AllProjectsName allProjects; + @Inject private GitRepositoryManager repoManager; + @Inject private IdentifiedUser.RequestFactory userFactory; + @Inject private InMemoryDatabase schemaFactory; + @Inject private LabelNormalizer norm; + @Inject private MetaDataUpdate.User metaDataUpdateFactory; + @Inject private ProjectCache projectCache; + @Inject private SchemaCreator schemaCreator; + + private LifecycleManager lifecycle; + private ReviewDb db; + private Account.Id userId; + private IdentifiedUser user; + private Change change; + + @Before + public void setUpInjector() throws Exception { + Injector injector = Guice.createInjector(new InMemoryModule()); + injector.injectMembers(this); + lifecycle = new LifecycleManager(); + lifecycle.add(injector); + lifecycle.start(); + + db = schemaFactory.open(); + schemaCreator.create(db); + userId = accountManager.authenticate(AuthRequest.forUser("user")) + .getAccountId(); + user = userFactory.create(userId); + + configureProject(); + setUpChange(); + } + + private void configureProject() throws Exception { + ProjectConfig pc = loadAllProjects(); + for (AccessSection sec : pc.getAccessSections()) { + for (String label : pc.getLabelSections().keySet()) { + sec.removePermission(forLabel(label)); + } + } + LabelType lt = category("Verified", + value(1, "Verified"), + value(0, "No score"), + value(-1, "Fails")); + pc.getLabelSections().put(lt.getName(), lt); + save(pc); + } + + private void setUpChange() throws Exception { + change = new Change( + new Change.Key("Iabcd1234abcd1234abcd1234abcd1234abcd1234"), + new Change.Id(1), userId, + new Branch.NameKey(allProjects, "refs/heads/master"), + TimeUtil.nowTs()); + ChangeUtil.computeSortKey(change); + PatchSetInfo ps = new PatchSetInfo(new PatchSet.Id(change.getId(), 1)); + ps.setSubject("Test change"); + change.setCurrentPatchSet(ps); + db.changes().insert(ImmutableList.of(change)); + } + + @After + public void tearDown() { + if (lifecycle != null) { + lifecycle.stop(); + } + if (db != null) { + db.close(); + } + InMemoryDatabase.drop(schemaFactory); + } + + @Test + public void normalizeByPermission() throws Exception { + ProjectConfig pc = loadAllProjects(); + grant(pc, forLabel("Code-Review"), -1, 1, REGISTERED_USERS, "refs/heads/*"); + grant(pc, forLabel("Verified"), -1, 1, REGISTERED_USERS, "refs/heads/*"); + save(pc); + + PatchSetApproval cr = psa(userId, "Code-Review", 2); + PatchSetApproval v = psa(userId, "Verified", 1); + assertEquals(ImmutableList.of(copy(cr, 1), v), + norm.normalize(change, ImmutableList.of(cr, v))); + } + + @Test + public void normalizeByType() throws Exception { + ProjectConfig pc = loadAllProjects(); + grant(pc, forLabel("Code-Review"), -5, 5, REGISTERED_USERS, "refs/heads/*"); + grant(pc, forLabel("Verified"), -5, 5, REGISTERED_USERS, "refs/heads/*"); + save(pc); + + PatchSetApproval cr = psa(userId, "Code-Review", 5); + PatchSetApproval v = psa(userId, "Verified", 5); + assertEquals(ImmutableList.of(copy(cr, 2), copy(v, 1)), + norm.normalize(change, ImmutableList.of(cr, v))); + } + + @Test + public void emptyPermissionRangeOmitsResult() throws Exception { + PatchSetApproval cr = psa(userId, "Code-Review", 1); + PatchSetApproval v = psa(userId, "Verified", 1); + assertEquals(ImmutableList.of(), + norm.normalize(change, ImmutableList.of(cr, v))); + } + + @Test + public void explicitZeroVoteOnNonEmptyRangeIsPresent() throws Exception { + ProjectConfig pc = loadAllProjects(); + grant(pc, forLabel("Code-Review"), -1, 1, REGISTERED_USERS, "refs/heads/*"); + save(pc); + + PatchSetApproval cr = psa(userId, "Code-Review", 0); + PatchSetApproval v = psa(userId, "Verified", 0); + assertEquals(ImmutableList.of(cr), + norm.normalize(change, ImmutableList.of(cr, v))); + } + + private ProjectConfig loadAllProjects() throws Exception { + Repository repo = repoManager.openRepository(allProjects); + try { + ProjectConfig pc = new ProjectConfig(allProjects); + pc.load(repo); + return pc; + } finally { + repo.close(); + } + } + + private void save(ProjectConfig pc) throws Exception { + MetaDataUpdate md = + metaDataUpdateFactory.create(pc.getProject().getNameKey(), user); + pc.commit(md); + projectCache.evict(pc.getProject().getNameKey()); + } + + private PatchSetApproval psa(Account.Id accountId, String label, int value) { + return new PatchSetApproval( + new PatchSetApproval.Key( + change.currentPatchSetId(), accountId, new LabelId(label)), + (short) value, TimeUtil.nowTs()); + } + + private PatchSetApproval copy(PatchSetApproval src, int newValue) { + PatchSetApproval result = + new PatchSetApproval(src.getKey().getParentKey(), src); + result.setValue((short) newValue); + return result; + } +}