diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java index 6e6ef374be..5c5746b81f 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java @@ -22,6 +22,7 @@ import static org.junit.Assert.assertEquals; import com.google.common.base.Joiner; import com.google.common.primitives.Chars; import com.google.gerrit.extensions.api.GerritApi; +import com.google.gerrit.extensions.api.changes.RevisionApi; import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.ListChangesOption; import com.google.gerrit.extensions.restapi.RestApiException; @@ -196,4 +197,10 @@ public abstract class AbstractDaemonTest { protected static Gson newGson() { return OutputFormat.JSON_COMPACT.newGson(); } + + protected RevisionApi revision(PushOneCommit.Result r) throws Exception { + return gApi.changes() + .id(r.getChangeId()) + .current(); + } } diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java new file mode 100644 index 0000000000..83c9f3822b --- /dev/null +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java @@ -0,0 +1,150 @@ +// 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.acceptance.server.project; + +import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_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 static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + +import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.acceptance.NoHttpd; +import com.google.gerrit.acceptance.PushOneCommit; +import com.google.gerrit.common.data.LabelType; +import com.google.gerrit.common.data.Permission; +import com.google.gerrit.extensions.api.changes.ReviewInput; +import com.google.gerrit.extensions.common.ChangeInfo; +import com.google.gerrit.extensions.common.LabelInfo; +import com.google.gerrit.reviewdb.client.AccountGroup; +import com.google.gerrit.server.config.AllProjectsName; +import com.google.gerrit.server.git.MetaDataUpdate; +import com.google.gerrit.server.git.ProjectConfig; +import com.google.gerrit.server.group.SystemGroupBackend; +import com.google.gerrit.server.project.ProjectCache; +import com.google.inject.Inject; + +import org.junit.Before; +import org.junit.Test; + +@NoHttpd +public class CustomLabelIT extends AbstractDaemonTest { + + @Inject + private ProjectCache projectCache; + + @Inject + private AllProjectsName allProjects; + + @Inject + private MetaDataUpdate.Server metaDataUpdateFactory; + + private final LabelType Q = category("CustomLabel", + value(1, "Positive"), + value(0, "No score"), + value(-1, "Negative")); + + @Before + public void setUp() throws Exception { + ProjectConfig cfg = projectCache.checkedGet(allProjects).getConfig(); + AccountGroup.UUID anonymousUsers = + SystemGroupBackend.getGroup(ANONYMOUS_USERS).getUUID(); + grant(cfg, Permission.forLabel(Q.getName()), -1, 1, anonymousUsers, + "refs/heads/*"); + saveProjectConfig(cfg); + } + + @Test + public void customLabelNoOp_NegativeVoteNotBlock() throws Exception { + Q.setFunctionName("NoOp"); + saveLabelConfig(); + PushOneCommit.Result r = createChange(); + revision(r).review(new ReviewInput().label(Q.getName(), -1)); + ChangeInfo c = get(r.getChangeId()); + LabelInfo q = c.labels.get(Q.getName()); + assertEquals(1, q.all.size()); + assertNull(q.rejected); + assertNotNull(q.disliked); + } + + @Test + public void customLabelNoBlock_NegativeVoteNotBlock() throws Exception { + Q.setFunctionName("NoBlock"); + saveLabelConfig(); + PushOneCommit.Result r = createChange(); + revision(r).review(new ReviewInput().label(Q.getName(), -1)); + ChangeInfo c = get(r.getChangeId()); + LabelInfo q = c.labels.get(Q.getName()); + assertEquals(1, q.all.size()); + assertNull(q.rejected); + assertNotNull(q.disliked); + } + + @Test + public void customLabelMaxNoBlock_NegativeVoteNotBlock() throws Exception { + Q.setFunctionName("MaxNoBlock"); + saveLabelConfig(); + PushOneCommit.Result r = createChange(); + revision(r).review(new ReviewInput().label(Q.getName(), -1)); + ChangeInfo c = get(r.getChangeId()); + LabelInfo q = c.labels.get(Q.getName()); + assertEquals(1, q.all.size()); + assertNull(q.rejected); + assertNotNull(q.disliked); + } + + @Test + public void customLabelAnyWithBlock_NegativeVoteBlock() throws Exception { + Q.setFunctionName("AnyWithBlock"); + saveLabelConfig(); + PushOneCommit.Result r = createChange(); + revision(r).review(new ReviewInput().label(Q.getName(), -1)); + ChangeInfo c = get(r.getChangeId()); + LabelInfo q = c.labels.get(Q.getName()); + assertEquals(1, q.all.size()); + assertNull(q.disliked); + assertNotNull(q.rejected); + } + + @Test + public void customLabelMaxWithBlock_NegativeVoteBlock() throws Exception { + saveLabelConfig(); + PushOneCommit.Result r = createChange(); + revision(r).review(new ReviewInput().label(Q.getName(), -1)); + ChangeInfo c = get(r.getChangeId()); + LabelInfo q = c.labels.get(Q.getName()); + assertEquals(1, q.all.size()); + assertNull(q.disliked); + assertNotNull(q.rejected); + } + + private void saveLabelConfig() throws Exception { + ProjectConfig cfg = projectCache.checkedGet(allProjects).getConfig(); + cfg.getLabelSections().put(Q.getName(), Q); + saveProjectConfig(cfg); + } + + private void saveProjectConfig(ProjectConfig cfg) throws Exception { + MetaDataUpdate md = metaDataUpdateFactory.create(allProjects); + try { + cfg.commit(md); + } finally { + md.close(); + } + projectCache.evict(allProjects); + } +} diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/LabelTypeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/LabelTypeIT.java index e94d8f3838..a2dd8ec0ab 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/LabelTypeIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/LabelTypeIT.java @@ -15,7 +15,6 @@ package com.google.gerrit.acceptance.server.project; import static com.google.common.base.Preconditions.checkNotNull; -import static com.google.gerrit.extensions.common.ListChangesOption.DETAILED_LABELS; import static org.junit.Assert.assertEquals; import com.google.gerrit.acceptance.AbstractDaemonTest; @@ -23,7 +22,6 @@ import com.google.gerrit.acceptance.NoHttpd; import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.common.data.LabelType; import com.google.gerrit.extensions.api.changes.ReviewInput; -import com.google.gerrit.extensions.api.changes.RevisionApi; import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.LabelInfo; import com.google.gerrit.server.config.AllProjectsName; @@ -235,12 +233,6 @@ public class LabelTypeIT extends AbstractDaemonTest { } } - private RevisionApi revision(PushOneCommit.Result r) throws Exception { - return gApi.changes() - .id(r.getChangeId()) - .current(); - } - private void merge(PushOneCommit.Result r) throws Exception { revision(r).review(ReviewInput.approve()); revision(r).submit(); @@ -261,7 +253,7 @@ public class LabelTypeIT extends AbstractDaemonTest { throws Exception { // Don't use asserts from PushOneCommit so we can test the round-trip // through JSON instead of querying the DB directly. - ChangeInfo c = get(r.getChangeId(), DETAILED_LABELS); + ChangeInfo c = get(r.getChangeId()); LabelInfo cr = c.labels.get("Code-Review"); assertEquals(1, cr.all.size()); assertEquals("Administrator", cr.all.get(0).name); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java index 6d2dc581a3..cb852a8666 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java @@ -482,18 +482,12 @@ public class ChangeJson { return; } - if (score != 0) { - if (score == type.getMin().getValue()) { - label.rejected = accountLoader.get(accountId); - } else if (score == type.getMax().getValue()) { - label.approved = accountLoader.get(accountId); - } else if (score < 0) { - label.disliked = accountLoader.get(accountId); - label.value = score; - } else if (score > 0 && label.disliked == null) { - label.recommended = accountLoader.get(accountId); - label.value = score; - } + if (score < 0) { + label.disliked = accountLoader.get(accountId); + label.value = score; + } else if (score > 0 && label.disliked == null) { + label.recommended = accountLoader.get(accountId); + label.value = score; } }