From 5b269c3c259edd96ce4f3983b67a5184fddbe445 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Sun, 2 Feb 2014 12:54:35 +0100 Subject: [PATCH 1/2] ChangeJson: Fix reviewed property initialization ChangeJson.reviewed property was not always initialized even though REVIEWED option was provided. This breaks plugin API. Change-Id: Ibe1cd1651d981424bcc7502a87b1f35308ba8828 --- .../main/java/com/google/gerrit/server/change/ChangeJson.java | 3 +++ 1 file changed, 3 insertions(+) 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 5da693df77..7f9c794306 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 @@ -226,6 +226,9 @@ public class ChangeJson { private ChangeInfo format(ChangeData cd, Optional limitToPsId) throws OrmException { accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS)); + if (has(REVIEWED)) { + ensureReviewedLoaded(Collections.singleton(cd)); + } ChangeInfo res = toChangeInfo(cd, limitToPsId); accountLoader.fill(); return res; From 03ff325a0dede70cc1b9c39ec427d410aa0484ec Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Sat, 1 Feb 2014 01:20:38 +0100 Subject: [PATCH 2/2] Change API: Add convenience get() and info() methods Add aliases to ChangeInfo get(EnumSet) method for frequently used change info retrieval operations: * get() with ListChangesOption set to ALL * info() with ListChangesOption set to NONE Change-Id: Ie9229923d1af39c34ca4dbffec8511e242051a9a --- .../rest/change/ChangeMessagesIT.java | 19 ++++++++++--------- .../extensions/api/changes/ChangeApi.java | 5 +++++ .../server/api/changes/ChangeApiImpl.java | 10 ++++++++++ .../server/api/changes/ChangeInfoMapper.java | 11 ++++++++--- 4 files changed, 33 insertions(+), 12 deletions(-) diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeMessagesIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeMessagesIT.java index fff1d1d4ba..f7a51665d4 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeMessagesIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeMessagesIT.java @@ -16,7 +16,6 @@ package com.google.gerrit.acceptance.rest.change; import static com.google.gerrit.acceptance.GitUtil.cloneProject; import static com.google.gerrit.acceptance.GitUtil.createProject; -import static com.google.gerrit.extensions.common.ListChangesOption.MESSAGES; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; @@ -29,7 +28,6 @@ import com.google.gerrit.extensions.api.GerritApi; import com.google.gerrit.extensions.api.changes.ReviewInput; import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.ChangeMessageInfo; -import com.google.gerrit.extensions.common.ListChangesOption; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.server.ReviewDb; @@ -45,7 +43,6 @@ import org.junit.Before; import org.junit.Test; import java.io.IOException; -import java.util.EnumSet; import java.util.Iterator; public class ChangeMessagesIT extends AbstractDaemonTest { @@ -90,8 +87,7 @@ public class ChangeMessagesIT extends AbstractDaemonTest { IOException, RestApiException { String changeId = createChange(); postMessage(changeId, "Some nits need to be fixed."); - ChangeInfo c = getChange("p~master~" + changeId, - EnumSet.noneOf(ListChangesOption.class)); + ChangeInfo c = getChange("p~master~" + changeId); assertNull(c.messages); } @@ -99,7 +95,7 @@ public class ChangeMessagesIT extends AbstractDaemonTest { public void defaultMessage() throws GitAPIException, IOException, RestApiException { String changeId = createChange(); - ChangeInfo c = getChange("p~master~" + changeId, EnumSet.of(MESSAGES)); + ChangeInfo c = getChangeAll("p~master~" + changeId); assertNotNull(c.messages); assertEquals(1, c.messages.size()); assertEquals("Uploaded patch set 1.", c.messages.iterator().next().message); @@ -113,7 +109,7 @@ public class ChangeMessagesIT extends AbstractDaemonTest { postMessage(changeId, firstMessage); String secondMessage = "I like this feature."; postMessage(changeId, secondMessage); - ChangeInfo c = getChange("p~master~" + changeId, EnumSet.of(MESSAGES)); + ChangeInfo c = getChangeAll("p~master~" + changeId); assertNotNull(c.messages); assertEquals(3, c.messages.size()); Iterator it = c.messages.iterator(); @@ -139,8 +135,13 @@ public class ChangeMessagesIT extends AbstractDaemonTest { .consume(); } - private ChangeInfo getChange(String triplet, EnumSet s) + private ChangeInfo getChange(String triplet) throws RestApiException { - return gApi.changes().id(triplet).get(s); + return gApi.changes().id(triplet).info(); + } + + private ChangeInfo getChangeAll(String triplet) + throws RestApiException { + return gApi.changes().id(triplet).get(); } } diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java index 3d06d7cb8a..f0a9e4d7f8 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java @@ -40,4 +40,9 @@ public interface ChangeApi { void addReviewer(String in) throws RestApiException; ChangeInfo get(EnumSet options) throws RestApiException; + + /** {@code get} with {@link ListChangesOption} set to ALL. */ + ChangeInfo get() throws RestApiException; + /** {@code get} with {@link ListChangesOption} set to NONE. */ + ChangeInfo info() throws RestApiException; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java index 0817250aef..0bcfd65188 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java @@ -170,4 +170,14 @@ class ChangeApiImpl implements ChangeApi { throw new RestApiException("Cannot retrieve change", e); } } + + @Override + public ChangeInfo get() throws RestApiException { + return get(EnumSet.allOf(ListChangesOption.class)); + } + + @Override + public ChangeInfo info() throws RestApiException { + return get(EnumSet.noneOf(ListChangesOption.class)); + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeInfoMapper.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeInfoMapper.java index 1459b03119..4faae5dac6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeInfoMapper.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeInfoMapper.java @@ -118,9 +118,11 @@ class ChangeInfoMapper { lo.value = li.value; lo.optional = li.optional; lo.values = li.values; - lo.all = Lists.newArrayListWithExpectedSize(li.all.size()); - for (ChangeJson.ApprovalInfo ai : li.all) { - lo.all.add(fromApprovalInfo(ai)); + if (li.all != null) { + lo.all = Lists.newArrayListWithExpectedSize(li.all.size()); + for (ChangeJson.ApprovalInfo ai : li.all) { + lo.all.add(fromApprovalInfo(ai)); + } } r.put(e.getKey(), lo); } @@ -141,6 +143,9 @@ class ChangeInfoMapper { private static AccountInfo fromAcountInfo( com.google.gerrit.server.account.AccountInfo i) { + if (i == null) { + return null; + } AccountInfo ai = new AccountInfo(); fromAccount(i, ai); return ai;