Merge branch 'stable-3.1' * stable-3.1: (31 commits) Shorten command to run tests Update documentation of where Bazel puts the jar Give an example of an API token in Documentation Drop `username` requirement from documentation Add missing facade tests Clarified message when comment adding failed Make implicit expectations about Conduit explicit in Facate tests Switch from `assertEquals` to Google's Truth Add tests for SearchUtils Add test for `projectSearch` with no match Add tests for `maniphestSearch` De-duplicate code for Phabricator result objects Simplify ManiphestSearch Drop the filtering for `id` from `maniphestSearch` Drop attachments from `maniphestSearch` Remove unneeded class Task Re-do and repair Conduit's maniphestEdit Simplify ProjectSearch Drop name filtering for project searches Make Conduit more compact through ImmutableList and ImmutableMap ... Change-Id: I0b889109276f41637dda142ccefe81e2cadd1076
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/PhabricatorItsFacade.java b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/PhabricatorItsFacade.java index df12578..c7e3fc6 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/PhabricatorItsFacade.java +++ b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/PhabricatorItsFacade.java
@@ -14,54 +14,45 @@ package com.googlesource.gerrit.plugins.its.phabricator; -import com.google.common.collect.Sets; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.extensions.annotations.PluginName; import com.google.gerrit.server.config.GerritServerConfig; -import com.google.gson.Gson; -import com.google.gson.JsonArray; -import com.google.gson.JsonElement; import com.google.inject.Inject; import com.googlesource.gerrit.plugins.its.base.its.ItsFacade; import com.googlesource.gerrit.plugins.its.phabricator.conduit.Conduit; -import com.googlesource.gerrit.plugins.its.phabricator.conduit.ConduitErrorException; import com.googlesource.gerrit.plugins.its.phabricator.conduit.ConduitException; -import com.googlesource.gerrit.plugins.its.phabricator.conduit.results.ManiphestResults; -import com.googlesource.gerrit.plugins.its.phabricator.conduit.results.ManiphestSearch; -import com.googlesource.gerrit.plugins.its.phabricator.conduit.results.ProjectSearch; import java.io.IOException; import java.net.URL; -import java.util.Set; import org.eclipse.jgit.lib.Config; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; public class PhabricatorItsFacade implements ItsFacade { - private static final Logger log = LoggerFactory.getLogger(PhabricatorItsFacade.class); + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final String GERRIT_CONFIG_URL = "url"; private static final String GERRIT_CONFIG_TOKEN = "token"; private final Conduit conduit; - private final Gson gson; @Inject - public PhabricatorItsFacade(@PluginName String pluginName, @GerritServerConfig Config cfg) { + public PhabricatorItsFacade( + @PluginName String pluginName, + @GerritServerConfig Config cfg, + Conduit.Factory conduitFactory) { String url = cfg.getString(pluginName, null, GERRIT_CONFIG_URL); String token = cfg.getString(pluginName, null, GERRIT_CONFIG_TOKEN); - this.conduit = new Conduit(url, token); - this.gson = new Gson(); + this.conduit = conduitFactory.create(url, token); } @Override public void addComment(final String bugId, final String comment) throws IOException { int task_id = Integer.parseInt(bugId); try { - conduit.maniphestEdit(task_id, comment); + conduit.maniphestEdit(task_id, comment, null, null); } catch (ConduitException e) { - throw new IOException("Could not update message for task " + task_id, e); + throw new IOException("Could not add comment for task " + task_id, e); } - log.debug("Added comment " + comment + " to bug " + task_id); + logger.atFine().log("Added comment %s to bug %s", comment, task_id); } @Override @@ -76,18 +67,7 @@ Boolean ret = false; int task_id = Integer.parseInt(bugId); try { - try { - conduit.maniphestSearch(task_id); - ret = true; - } catch (ConduitErrorException e) { - // An ERR_BAD_TASK just means that the task does not exist. - // So the default value of ret would be ok - if (!("ERR_BAD_TASK".equals(e.getErrorCode()))) { - // So we had an exception that is /not/ ERR_BAD_TASK. - // We have to relay that to the caller. - throw e; - } - } + ret = (conduit.maniphestSearch(task_id) != null); } catch (ConduitException e) { throw new IOException("Could not check existence of task " + task_id, e); } @@ -101,19 +81,21 @@ String chopped[] = actionString.split(" "); if (chopped.length >= 1) { String action = chopped[0]; - switch (action) { - case "add-project": - assertParameters(action, chopped, 1); - - maniphestEdit(chopped[1], taskId, Conduit.ACTION_PROJECT_ADD); - break; - case "remove-project": - assertParameters(action, chopped, 1); - - maniphestEdit(chopped[1], taskId, Conduit.ACTION_PROJECT_REMOVE); - break; - default: - throw new IOException("Unknown action " + action); + try { + switch (action) { + case "add-project": + assertParameters(action, chopped, 1); + conduit.maniphestEdit(taskId, null, chopped[1], null); + break; + case "remove-project": + assertParameters(action, chopped, 1); + conduit.maniphestEdit(taskId, null, null, chopped[1]); + break; + default: + throw new IOException("Unknown action " + action); + } + } catch (ConduitException e) { + throw new IOException("Could not perform action " + action, e); } } else { throw new IOException("Could not parse action " + actionString); @@ -129,35 +111,6 @@ } } - private void maniphestEdit(String projectName, int taskId, String actions) throws IOException { - try { - ProjectSearch projectSearch = conduit.projectSearch(projectName); - String projectPhid = projectSearch.getPhid(); - - Set<String> projectPhids = Sets.newHashSet(projectPhid); - - ManiphestResults taskSearch = conduit.maniphestSearch(taskId); - JsonArray maniphestResultEntryValue = taskSearch.getData().getAsJsonArray(); - - for (JsonElement jsonElement : maniphestResultEntryValue) { - ManiphestSearch maniphestResultManiphestSearch = - gson.fromJson(jsonElement, ManiphestSearch.class); - for (JsonElement jsonElement2 : - maniphestResultManiphestSearch - .getAttachments() - .getProjects() - .getProjectPHIDs() - .getAsJsonArray()) { - projectPhids.add(jsonElement2.getAsString()); - } - } - - conduit.maniphestEdit(taskId, projectPhids, actions); - } catch (ConduitException e) { - throw new IOException("Error on conduit", e); - } - } - @Override public String healthCheck(final Check check) throws IOException { // This method is not used, so there is no need to implement it.
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/PhabricatorModule.java b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/PhabricatorModule.java index 847e7bb..402316e 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/PhabricatorModule.java +++ b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/PhabricatorModule.java
@@ -14,22 +14,24 @@ package com.googlesource.gerrit.plugins.its.phabricator; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.extensions.annotations.PluginName; +import com.google.gerrit.extensions.config.FactoryModule; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.PluginConfigFactory; -import com.google.inject.AbstractModule; import com.google.inject.Inject; +import com.google.inject.Scopes; import com.googlesource.gerrit.plugins.its.base.ItsHookModule; import com.googlesource.gerrit.plugins.its.base.its.ItsFacade; import com.googlesource.gerrit.plugins.its.base.its.ItsFacadeFactory; import com.googlesource.gerrit.plugins.its.base.its.SingleItsServer; +import com.googlesource.gerrit.plugins.its.phabricator.conduit.Conduit; +import com.googlesource.gerrit.plugins.its.phabricator.conduit.ConduitConnection; import org.eclipse.jgit.lib.Config; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -public class PhabricatorModule extends AbstractModule { +public class PhabricatorModule extends FactoryModule { - private static final Logger log = LoggerFactory.getLogger(PhabricatorModule.class); + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private final String pluginName; private final Config gerritConfig; @@ -48,8 +50,10 @@ @Override protected void configure() { if (gerritConfig.getString(pluginName, null, "url") != null) { - log.info("Phabricator is configured as ITS"); - bind(ItsFacade.class).toInstance(new PhabricatorItsFacade(pluginName, gerritConfig)); + logger.atInfo().log("Phabricator is configured as ITS"); + factory(ConduitConnection.Factory.class); + factory(Conduit.Factory.class); + bind(ItsFacade.class).to(PhabricatorItsFacade.class).in(Scopes.SINGLETON); bind(ItsFacadeFactory.class).to(SingleItsServer.class); install(new ItsHookModule(pluginName, pluginCfgFactory));
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/Conduit.java b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/Conduit.java index 33bdf7e..a8e9d69 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/Conduit.java +++ b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/Conduit.java
@@ -14,21 +14,22 @@ package com.googlesource.gerrit.plugins.its.phabricator.conduit; +import com.google.common.base.Strings; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.gson.Gson; -import com.google.gson.JsonArray; import com.google.gson.JsonElement; import com.google.gson.JsonObject; +import com.google.inject.Inject; +import com.google.inject.assistedinject.Assisted; import com.googlesource.gerrit.plugins.its.phabricator.conduit.results.ConduitPing; import com.googlesource.gerrit.plugins.its.phabricator.conduit.results.ManiphestEdit; -import com.googlesource.gerrit.plugins.its.phabricator.conduit.results.ManiphestResults; -import com.googlesource.gerrit.plugins.its.phabricator.conduit.results.ProjectResults; +import com.googlesource.gerrit.plugins.its.phabricator.conduit.results.ManiphestSearch; import com.googlesource.gerrit.plugins.its.phabricator.conduit.results.ProjectSearch; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * Bindings for Phabricator's Conduit API @@ -36,6 +37,9 @@ * <p>This class is not thread-safe. */ public class Conduit { + public interface Factory { + Conduit create(@Assisted("baseUrl") String baseUrl, @Assisted("token") String token); + } public static final String ACTION_COMMENT = "comment"; @@ -43,29 +47,25 @@ public static final String ACTION_PROJECT_REMOVE = "projects.remove"; - private static final Logger log = LoggerFactory.getLogger(Conduit.class); - public static final int CONDUIT_VERSION = 7; + private final SearchUtils searchUtils; private final ConduitConnection conduitConnection; private final Gson gson; + private final String token; - private String token; - - public Conduit(final String baseUrl) { - this(baseUrl, null); - } - - public Conduit(final String baseUrl, final String token) { - this.conduitConnection = new ConduitConnection(baseUrl); + @Inject + public Conduit( + ConduitConnection.Factory conduitConnectionFactory, + SearchUtils searchUtils, + @Assisted("baseUrl") String baseUrl, + @Assisted("token") String token) { + this.searchUtils = searchUtils; + this.conduitConnection = conduitConnectionFactory.create(baseUrl); this.token = token; this.gson = new Gson(); } - public void setToekn(String token) { - this.token = token; - } - /** Runs the API's 'conduit.ping' method */ public ConduitPing conduitPing() throws ConduitException { Map<String, Object> params = new HashMap<>(); @@ -77,93 +77,62 @@ } /** Runs the API's 'maniphest.search' method */ - public ManiphestResults maniphestSearch(int taskId) throws ConduitException { + public ManiphestSearch maniphestSearch(int taskId) throws ConduitException { HashMap<String, Object> params = new HashMap<>(); - HashMap<String, Object> params2 = new HashMap<>(); - HashMap<String, Object> params3 = new HashMap<>(); - - List<Object> list = new ArrayList<>(); - list.add(taskId); - - params2.put("ids", list); - - params.put("constraints", params2); - - params3.put("projects", true); - params.put("attachments", params3); + params.put("constraints", ImmutableMap.of("ids", ImmutableList.of(taskId))); JsonElement callResult = conduitConnection.call("maniphest.search", params, token); - ManiphestResults result = gson.fromJson(callResult, ManiphestResults.class); - return result; - } - - /** Runs the API's 'maniphest.edit' method */ - public ManiphestEdit maniphestEdit(int taskId, String comment) throws ConduitException { - return maniphestEdit(taskId, comment, null, ACTION_COMMENT); - } - - /** Runs the API's 'maniphest.edit' method */ - public ManiphestEdit maniphestEdit(int taskId, Iterable<String> projects, String action) - throws ConduitException { - return maniphestEdit(taskId, null, projects, action); + return searchUtils.stream(callResult, ManiphestSearch.class).findFirst().orElse(null); } /** Runs the API's 'maniphest.edit' method */ public ManiphestEdit maniphestEdit( - int taskId, String comment, Iterable<String> projects, String action) + int taskId, String comment, String projectNameToAdd, String projectNameToRemove) throws ConduitException { - HashMap<String, Object> params = new HashMap<>(); - List<Object> list = new ArrayList<>(); - HashMap<String, Object> params2 = new HashMap<>(); - params2.put("type", action); - if (action.equals(ACTION_COMMENT)) { - if (comment == null) { - throw new IllegalArgumentException( - "The value of comment (null) is invalid for the action" + action); - } - params2.put("value", comment); + ManiphestEdit result = null; + List<Object> transactions = new ArrayList<>(); + + if (!Strings.isNullOrEmpty(comment)) { + HashMap<String, Object> transaction = new HashMap<>(); + transaction.put("type", ACTION_COMMENT); + transaction.put("value", comment); + + transactions.add(transaction); } - if (action.equals(ACTION_PROJECT_ADD) || action.equals(ACTION_PROJECT_REMOVE)) { - if ((action.equals(ACTION_PROJECT_ADD) || action.equals(ACTION_PROJECT_REMOVE)) - && projects == null) { - throw new IllegalArgumentException( - "The value of projects (null) is invalid for the action " + action); - } - params2.put("value", projects); + if (!Strings.isNullOrEmpty(projectNameToAdd)) { + HashMap<String, Object> transaction = new HashMap<>(); + transaction.put("type", ACTION_PROJECT_ADD); + transaction.put("value", ImmutableList.of(projectSearch(projectNameToAdd).getPhid())); + + transactions.add(transaction); } - if (!params2.isEmpty()) { - list.add(params2); - params.put("transactions", list); - } - params.put("objectIdentifier", taskId); + if (!Strings.isNullOrEmpty(projectNameToRemove)) { + HashMap<String, Object> transaction = new HashMap<>(); + transaction.put("type", ACTION_PROJECT_REMOVE); + transaction.put("value", ImmutableList.of(projectSearch(projectNameToRemove).getPhid())); - JsonElement callResult = conduitConnection.call("maniphest.edit", params, token); - ManiphestEdit result = gson.fromJson(callResult, ManiphestEdit.class); + transactions.add(transaction); + } + + if (!transactions.isEmpty()) { + HashMap<String, Object> params = new HashMap<>(); + params.put("objectIdentifier", taskId); + params.put("transactions", transactions); + JsonElement callResult = conduitConnection.call("maniphest.edit", params, token); + result = gson.fromJson(callResult, ManiphestEdit.class); + } + return result; } - /** Runs the API's 'projectSearch' method to match exactly one project name */ + /** Runs the API's 'project.search' method to match exactly one project name */ public ProjectSearch projectSearch(String name) throws ConduitException { HashMap<String, Object> params = new HashMap<>(); - HashMap<String, Object> params2 = new HashMap<>(); - - params2.put("query", name); - - params.put("constraints", params2); + params.put("constraints", ImmutableMap.of("query", name)); JsonElement callResult = conduitConnection.call("project.search", params, token); - ProjectResults projectResult = gson.fromJson(callResult, ProjectResults.class); - JsonArray projectResultData = projectResult.getData().getAsJsonArray(); - - ProjectSearch result = null; - for (JsonElement jsonElement : projectResultData) { - ProjectSearch projectResultSearch = gson.fromJson(jsonElement, ProjectSearch.class); - if (projectResultSearch.getFields().getName().equals(name)) { - result = projectResultSearch; - } - } - return result; + return searchUtils.stream(callResult, ProjectSearch.class).findFirst().orElse(null); } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/ConduitConnection.java b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/ConduitConnection.java index 9d93080..8eb695b 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/ConduitConnection.java +++ b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/ConduitConnection.java
@@ -14,8 +14,11 @@ package com.googlesource.gerrit.plugins.its.phabricator.conduit; +import com.google.common.flogger.FluentLogger; import com.google.gson.Gson; import com.google.gson.JsonElement; +import com.google.inject.Inject; +import com.google.inject.assistedinject.Assisted; import com.googlesource.gerrit.plugins.its.phabricator.conduit.results.CallCapsule; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -28,19 +31,22 @@ import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClients; import org.apache.http.util.EntityUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** Abstracts the connection to Conduit API */ -class ConduitConnection { - private static final Logger log = LoggerFactory.getLogger(Conduit.class); +public class ConduitConnection { + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + + public interface Factory { + ConduitConnection create(String baseUrl); + } private final String apiUrlBase; private final Gson gson; private CloseableHttpClient client; - ConduitConnection(final String baseUrl) { + @Inject + ConduitConnection(@Assisted String baseUrl) { apiUrlBase = baseUrl.replaceAll("/+$", "") + "/api/"; gson = new Gson(); client = null; @@ -55,7 +61,7 @@ */ private CloseableHttpClient getClient() { if (client == null) { - log.trace("Creating new client connection"); + logger.atFinest().log("Creating new client connection"); client = HttpClients.createDefault(); } return client; @@ -94,17 +100,11 @@ String json = gson.toJson(params); - log.trace("Calling phabricator method " + method + " with the parameters " + json); + logger.atFinest().log("Calling phabricator method %s with the parameters %s", method, json); httppost.setEntity(new StringEntity("params=" + json, StandardCharsets.UTF_8)); - CloseableHttpResponse response; - try { - response = getClient().execute(httppost); - } catch (IOException e) { - throw new ConduitException("Could not execute Phabricator API call", e); - } - try { - log.trace("Phabricator HTTP response status: " + response.getStatusLine()); + try (CloseableHttpResponse response = getClient().execute(httppost)) { + logger.atFinest().log("Phabricator HTTP response status: %s", response.getStatusLine()); HttpEntity entity = response.getEntity(); String entityString; try { @@ -113,22 +113,18 @@ throw new ConduitException("Could not read the API response", e); } - log.trace("Phabricator response " + entityString); + logger.atFinest().log("Phabricator response: %s", entityString); CallCapsule callCapsule = gson.fromJson(entityString, CallCapsule.class); - log.trace("callCapsule.result: " + callCapsule.getResult()); - log.trace("callCapsule.error_code: " + callCapsule.getErrorCode()); - log.trace("callCapsule.error_info: " + callCapsule.getErrorInfo()); + logger.atFinest().log("callCapsule.result: %s", callCapsule.getResult()); + logger.atFinest().log("callCapsule.error_code: %s", callCapsule.getErrorCode()); + logger.atFinest().log("callCapsule.error_info: %s", callCapsule.getErrorInfo()); if (callCapsule.getErrorCode() != null || callCapsule.getErrorInfo() != null) { throw new ConduitErrorException( method, callCapsule.getErrorCode(), callCapsule.getErrorInfo()); } return callCapsule.getResult(); - } finally { - try { - response.close(); - } catch (IOException e) { - throw new ConduitException("Could not close API response", e); - } + } catch (IOException e) { + throw new ConduitException("Could not execute Phabricator API call", e); } } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/SearchUtils.java b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/SearchUtils.java new file mode 100644 index 0000000..0cfaf5c --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/SearchUtils.java
@@ -0,0 +1,36 @@ +// Copyright (C) 2017 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.googlesource.gerrit.plugins.its.phabricator.conduit; + +import com.google.common.collect.Streams; +import com.google.gson.Gson; +import com.google.gson.JsonElement; +import com.google.inject.Inject; +import com.googlesource.gerrit.plugins.its.phabricator.conduit.results.GenericSearch; +import java.util.stream.Stream; + +public class SearchUtils { + private final Gson gson; + + @Inject + public SearchUtils() { + gson = new Gson(); + } + + public <T> Stream<T> stream(JsonElement jsonResult, Class<T> classOfT) { + GenericSearch result = gson.fromJson(jsonResult, GenericSearch.class); + return Streams.stream(result.getData()).map((json) -> gson.fromJson(json, classOfT)); + } +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/GenericEdit.java b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/GenericEdit.java new file mode 100644 index 0000000..94105c3 --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/GenericEdit.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.googlesource.gerrit.plugins.its.phabricator.conduit.results; + +import java.util.List; + +/** + * Models the result for API methods + * + * <p>JSON looks like: + * + * <pre> + * { + * "object":{ + * "id":2, + * "phid":"PHID-TASK-wzydcwamkp5rjhg45ocq" + * }, + * "transactions":[ + * {"phid":"PHID-XACT-TASK-sghfp7saytwmun3"} + * ] + * } + * </pre> + */ +public class GenericEdit { + private ResultObject object; + private List<Transaction> transactions; + + public ResultObject getObject() { + return object; + } + + public List<Transaction> getTransactions() { + return transactions; + } + + public class Transaction extends PhabObject {} + + public class ResultObject extends PhabObjectWithId {} +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/ManiphestResults.java b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/GenericSearch.java similarity index 88% rename from src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/ManiphestResults.java rename to src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/GenericSearch.java index 8a28795..c911df8 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/ManiphestResults.java +++ b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/GenericSearch.java
@@ -14,7 +14,7 @@ package com.googlesource.gerrit.plugins.its.phabricator.conduit.results; -import com.google.gson.JsonElement; +import com.google.gson.JsonArray; /** * Models the result for API methods @@ -39,10 +39,10 @@ * } * </pre> */ -public class ManiphestResults { - private JsonElement data; +public class GenericSearch { + private JsonArray data; - public JsonElement getData() { + public JsonArray getData() { return data; } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/ManiphestEdit.java b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/ManiphestEdit.java index e9e3805..dc416dd 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/ManiphestEdit.java +++ b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/ManiphestEdit.java
@@ -20,31 +20,14 @@ * * <pre> * { - * "id":"48", - * "phid":"PHID-TASK-pemd324eosnymq3tdkyo", - * "authorPHID":"PHID-USER-na3one2sht11aone", - * "ownerPHID":null, - * "ccPHIDs":[ - * "PHID-USER-h4n62fq2kt2v3a2qjyqh" - * ], - * "status":"open", - * "statusName":"Open", - * "isClosed":false, - * "priority": "Needs Triage", - * "priorityColor":"violet", - * "title":"QChris test task", - * "description":"", - * "projectPHIDs":[], - * "uri":"https://phab-01.wmflabs.org/T47", - * "auxiliary":{ - * "std:maniphest:security_topic":"default", - * "isdc:sprint:storypoints":null + * "object":{ + * "id":2, + * "phid":"PHID-TASK-wzydcwamkp5rjhg45ocq" * }, - * "objectName":"T47", - * "dateCreated":"1413484594", - * "dateModified":1413549869, - * "dependsOnTaskPHIDs":[] + * "transactions":[ + * {"phid":"PHID-XACT-TASK-sghfp7saytwmun3"} + * ] * } * </pre> */ -public class ManiphestEdit extends Task {} +public class ManiphestEdit extends GenericEdit {}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/ManiphestSearch.java b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/ManiphestSearch.java index 1f98709..48a81e1 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/ManiphestSearch.java +++ b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/ManiphestSearch.java
@@ -14,8 +14,6 @@ package com.googlesource.gerrit.plugins.its.phabricator.conduit.results; -import com.google.gson.JsonElement; - /** * Models the result for a call to maniphest.search * @@ -67,36 +65,4 @@ * } * </pre> */ -public class ManiphestSearch { - private int id; - private JsonElement fields; - private Attachments attachments; - - public int getId() { - return id; - } - - public JsonElement getFields() { - return fields; - } - - public Attachments getAttachments() { - return attachments; - } - - public class Attachments { - private Projects projects; - - public Projects getProjects() { - return projects; - } - } - - public class Projects { - private JsonElement projectPHIDs; - - public JsonElement getProjectPHIDs() { - return projectPHIDs; - } - } -} +public class ManiphestSearch extends PhabObjectWithId {}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/PhabObject.java b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/PhabObject.java new file mode 100644 index 0000000..90c4819 --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/PhabObject.java
@@ -0,0 +1,31 @@ +// 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.googlesource.gerrit.plugins.its.phabricator.conduit.results; + +public class PhabObject { + private String phid; + + public PhabObject() { + this(null); + } + + public PhabObject(String phid) { + this.phid = phid; + } + + public String getPhid() { + return phid; + } +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/PhabObjectWithId.java b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/PhabObjectWithId.java new file mode 100644 index 0000000..dd8c1b9 --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/PhabObjectWithId.java
@@ -0,0 +1,32 @@ +// 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.googlesource.gerrit.plugins.its.phabricator.conduit.results; + +public class PhabObjectWithId extends PhabObject { + private int id; + + public PhabObjectWithId() { + super(); + } + + public PhabObjectWithId(String phid, int id) { + super(phid); + this.id = id; + } + + public int getId() { + return id; + } +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/ProjectResults.java b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/ProjectResults.java deleted file mode 100644 index fcd9b44..0000000 --- a/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/ProjectResults.java +++ /dev/null
@@ -1,48 +0,0 @@ -// Copyright (C) 2018 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.googlesource.gerrit.plugins.its.phabricator.conduit.results; - -import com.google.gson.JsonElement; - -/** - * Models the result for API methods - * - * <p>JSON looks like: - * - * <pre> - * { - * "data": [ - * { ... } - * ], - * "maps": {}, - * "query": { - * "queryKey": null - * }, - * "cursor": { - * "limit": 100, - * "after": null, - * "before": null, - * "order": null - * } - * } - * </pre> - */ -public class ProjectResults { - private JsonElement data; - - public JsonElement getData() { - return data; - } -}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/ProjectSearch.java b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/ProjectSearch.java index 82f2196..b4c3d53 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/ProjectSearch.java +++ b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/ProjectSearch.java
@@ -14,7 +14,6 @@ package com.googlesource.gerrit.plugins.its.phabricator.conduit.results; - /** * Models the result for API methods returning Project searches. * @@ -57,33 +56,8 @@ * } * </pre> */ -public class ProjectSearch { - private int id; - private String type; - private String phid; - private Fields fields; - - public int getId() { - return id; - } - - public String getType() { - return type; - } - - public String getPhid() { - return phid; - } - - public Fields getFields() { - return fields; - } - - public class Fields { - private String name; - - public String getName() { - return name; - } +public class ProjectSearch extends PhabObjectWithId { + public ProjectSearch(String phid, int id) { + super(phid, id); } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/Task.java b/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/Task.java deleted file mode 100644 index 177968d..0000000 --- a/src/main/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/results/Task.java +++ /dev/null
@@ -1,158 +0,0 @@ -// 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.googlesource.gerrit.plugins.its.phabricator.conduit.results; - -import com.google.gson.JsonElement; - -/** - * Models the result for API methods returning Task information - * - * <p>JSON looks like: - * - * <pre> - * { - * "id":"48", - * "phid":"PHID-TASK-pemd324eosnymq3tdkyo", - * "authorPHID":"PHID-USER-na3one2sht11aone", - * "ownerPHID":null, - * "ccPHIDs":[ - * "PHID-USER-h4n62fq2kt2v3a2qjyqh" - * ], - * "status":"open", - * "statusName":"Open", - * "isClosed":false, - * "priority": "Needs Triage", - * "priorityColor":"violet", - * "title":"QChris test task", - * "description":"", - * "projectPHIDs":[], - * "uri":"https://phab-01.wmflabs.org/T47", - * "auxiliary":{ - * "std:maniphest:security_topic":"default", - * "isdc:sprint:storypoints":null - * }, - * "objectName":"T47", - * "dateCreated":"1413484594", - * "dateModified":1413549869, - * "dependsOnTaskPHIDs":[] - * } - * </pre> - */ -public class Task { - private int id; - private String phid; - private String authorPHID; - private String ownerPHID; - private JsonElement ccPHIDs; - private String status; - private String statusName; - private Boolean isClosed; - private String priority; - private String priorityColor; - private String title; - private String description; - private JsonElement projectPHIDs; - private String uri; - private JsonElement auxiliary; - private String objectName; - private String dateCreated; - private String dateModified; - private JsonElement dependsOnTaskPHIDs; - private JsonElement objectIdentifier; - private JsonElement transactions; - - public int getId() { - return id; - } - - public String getPhid() { - return phid; - } - - public String getAuthorPHID() { - return authorPHID; - } - - public String getOwnerPHID() { - return ownerPHID; - } - - public JsonElement getCcPHIDs() { - return ccPHIDs; - } - - public String getStatus() { - return status; - } - - public String getStatusName() { - return statusName; - } - - public Boolean getIsClosed() { - return isClosed; - } - - public String getPriority() { - return priority; - } - - public String getPriorityColor() { - return priorityColor; - } - - public String getTitle() { - return title; - } - - public String getDescription() { - return description; - } - - public JsonElement getProjectPHIDs() { - return projectPHIDs; - } - - public String getUri() { - return uri; - } - - public JsonElement getAuxiliary() { - return auxiliary; - } - - public String getObjectName() { - return objectName; - } - - public String getDateCreated() { - return dateCreated; - } - - public String getDateModified() { - return dateModified; - } - - public JsonElement getDependsOnTaskPHIDs() { - return dependsOnTaskPHIDs; - } - - public JsonElement getObjectIdentifier() { - return objectIdentifier; - } - - public JsonElement getTransactions() { - return transactions; - } -}
diff --git a/src/main/resources/Documentation/build.md b/src/main/resources/Documentation/build.md index 2d18690..a90457c 100644 --- a/src/main/resources/Documentation/build.md +++ b/src/main/resources/Documentation/build.md
@@ -18,7 +18,7 @@ The output is created in ``` - bazel-genfiles/plugins/@PLUGIN@/@PLUGIN@.jar + bazel-bin/plugins/@PLUGIN@/@PLUGIN@.jar ``` This project can be imported into the Eclipse IDE, @@ -32,7 +32,7 @@ To execute the tests run: ``` - bazel test plugins/@PLUGIN@:its_phabricator_tests + bazel test plugins/@PLUGIN@:all ``` [Back to @PLUGIN@ documentation index][index]
diff --git a/src/main/resources/Documentation/config-connectivity.md b/src/main/resources/Documentation/config-connectivity.md index daa947a..84bc67f 100644 --- a/src/main/resources/Documentation/config-connectivity.md +++ b/src/main/resources/Documentation/config-connectivity.md
@@ -2,7 +2,7 @@ ======================== In order for @PLUGIN@ to connect to your Phabricator instance, url (without -trailing “/api”, “/conduit” or some such), user, and certificate are required in +trailing “/api”, “/conduit” or some such), and certificate are required in your site's `etc/gerrit.config` or `etc/secure.config` under the `@PLUGIN@` section. @@ -11,10 +11,12 @@ ``` [@PLUGIN@] url = http://my.phabricator.instance.example.org - token = TOKEN_FOR_ABOVE_USERNAME + token = TOKEN_AS_DESCRIBED_BELOW ``` You can get your token by going to http://my.phabricator.instance.example.org/conduit/login/ +Tokens typically start in `cli-` and are followed by letters and digits, as +for example `cli-zoenau772kfsrofqxt7cn55q4rng`. [Back to @PLUGIN@ documentation index][index]
diff --git a/src/test/java/com/googlesource/gerrit/plugins/its/phabricator/PhabricatorItsFacadeTest.java b/src/test/java/com/googlesource/gerrit/plugins/its/phabricator/PhabricatorItsFacadeTest.java index c164390..31ec0a5 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/its/phabricator/PhabricatorItsFacadeTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/its/phabricator/PhabricatorItsFacadeTest.java
@@ -13,7 +13,17 @@ // limitations under the License. package com.googlesource.gerrit.plugins.its.phabricator; -import static org.easymock.EasyMock.expect; +import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; import com.google.gerrit.extensions.annotations.PluginName; import com.google.gerrit.extensions.config.FactoryModule; @@ -21,78 +31,235 @@ import com.google.inject.Guice; import com.google.inject.Injector; import com.googlesource.gerrit.plugins.its.base.testutil.LoggingMockingTestCase; +import com.googlesource.gerrit.plugins.its.phabricator.conduit.Conduit; +import com.googlesource.gerrit.plugins.its.phabricator.conduit.ConduitException; +import com.googlesource.gerrit.plugins.its.phabricator.conduit.results.ManiphestEdit; +import com.googlesource.gerrit.plugins.its.phabricator.conduit.results.ManiphestSearch; +import java.io.IOException; +import java.net.URL; import org.eclipse.jgit.lib.Config; +import org.junit.Test; +import org.mockito.ArgumentCaptor; public class PhabricatorItsFacadeTest extends LoggingMockingTestCase { private Injector injector; private Config serverConfig; + private Conduit conduit; + private Conduit.Factory conduitFactory; + @Test public void testCreateLinkForWebUiDifferentUrlAndText() { - mockUnconnectablePhabricator(); - - replayMocks(); - PhabricatorItsFacade itsFacade = createPhabricatorItsFacade(); String actual = itsFacade.createLinkForWebui("Test-Url", "Test-Text"); - assertEquals("[[Test-Url|Test-Text]]", actual); + assertThat(actual).isEqualTo("[[Test-Url|Test-Text]]"); + + verifyZeroInteractions(conduit); } + @Test public void testCreateLinkForWebUiSameUrlAndText() { - mockUnconnectablePhabricator(); - - replayMocks(); - PhabricatorItsFacade itsFacade = createPhabricatorItsFacade(); String actual = itsFacade.createLinkForWebui("Test-Url", "Test-Url"); - assertEquals("[[Test-Url]]", actual); + assertThat(actual).isEqualTo("[[Test-Url]]"); + + verifyZeroInteractions(conduit); } + @Test public void testCreateLinkForWebUiNullText() { - mockUnconnectablePhabricator(); - - replayMocks(); - PhabricatorItsFacade itsFacade = createPhabricatorItsFacade(); String actual = itsFacade.createLinkForWebui("Test-Url", null); - assertEquals("[[Test-Url]]", actual); + assertThat(actual).isEqualTo("[[Test-Url]]"); + + verifyZeroInteractions(conduit); } + @Test public void testCreateLinkForWebUiEmptyText() { - mockUnconnectablePhabricator(); - - replayMocks(); - PhabricatorItsFacade itsFacade = createPhabricatorItsFacade(); String actual = itsFacade.createLinkForWebui("Test-Url", ""); - assertEquals("[[Test-Url]]", actual); + assertThat(actual).isEqualTo("[[Test-Url]]"); + + verifyZeroInteractions(conduit); + } + + @Test + public void testAddCommentPlain() throws Exception { + ManiphestEdit result = new ManiphestEdit(); + when(conduit.maniphestEdit(4711, "bar", null, null)).thenReturn(result); + + PhabricatorItsFacade itsFacade = createPhabricatorItsFacade(); + itsFacade.addComment("4711", "bar"); + + verify(conduit).maniphestEdit(4711, "bar", null, null); + verifyNoMoreInteractions(conduit); + + assertLogMessageContains("comment"); + } + + @Test + public void testAddCommentPlainNoNumber() throws Exception { + PhabricatorItsFacade itsFacade = createPhabricatorItsFacade(); + assertThrows(RuntimeException.class, () -> itsFacade.addComment("foo", "bar")); + + verifyZeroInteractions(conduit); + } + + @Test + public void testAddCommentConduitException() throws Exception { + when(conduit.maniphestEdit(4711, "bar", null, null)).thenThrow(new ConduitException()); + + PhabricatorItsFacade itsFacade = createPhabricatorItsFacade(); + assertThrows(IOException.class, () -> itsFacade.addComment("4711", "bar")); + + verify(conduit).maniphestEdit(4711, "bar", null, null); + verifyNoMoreInteractions(conduit); + } + + @Test + public void testAddRelatedLinkPlain() throws Exception { + ManiphestEdit result = new ManiphestEdit(); + when(conduit.maniphestEdit(anyInt(), anyString(), isNull(), isNull())).thenReturn(result); + + PhabricatorItsFacade itsFacade = createPhabricatorItsFacade(); + itsFacade.addRelatedLink("4711", new URL("http://related.example.org"), "description"); + + ArgumentCaptor<String> commentCapture = ArgumentCaptor.forClass(String.class); + verify(conduit).maniphestEdit(eq(4711), commentCapture.capture(), isNull(), isNull()); + verifyNoMoreInteractions(conduit); + + assertThat(commentCapture.getValue()).contains("[[http://related.example.org|description]]"); + + assertLogMessageContains("comment"); + } + + @Test + public void testExistsNumberExists() throws Exception { + when(conduit.maniphestSearch(4711)).thenReturn(new ManiphestSearch()); + + PhabricatorItsFacade itsFacade = createPhabricatorItsFacade(); + Boolean actual = itsFacade.exists("4711"); + + assertThat(actual).isTrue(); + + verify(conduit).maniphestSearch(4711); + verifyNoMoreInteractions(conduit); + } + + @Test + public void testExistsNumberDoesNotExist() throws Exception { + when(conduit.maniphestSearch(4711)).thenReturn(null); + + PhabricatorItsFacade itsFacade = createPhabricatorItsFacade(); + Boolean actual = itsFacade.exists("4711"); + + assertThat(actual).isFalse(); + + verify(conduit).maniphestSearch(4711); + verifyNoMoreInteractions(conduit); + } + + @Test + public void testExistsNumberConduitException() throws Exception { + when(conduit.maniphestSearch(4711)).thenThrow(new ConduitException()); + + PhabricatorItsFacade itsFacade = createPhabricatorItsFacade(); + assertThrows(IOException.class, () -> itsFacade.exists("4711")); + + verify(conduit).maniphestSearch(4711); + verifyNoMoreInteractions(conduit); + } + + @Test + public void testExistsNoNumber() throws Exception { + PhabricatorItsFacade itsFacade = createPhabricatorItsFacade(); + assertThrows(RuntimeException.class, () -> itsFacade.exists("foo")); + + verifyZeroInteractions(conduit); + } + + @Test + public void testPerformActionNoNumber() throws Exception { + PhabricatorItsFacade itsFacade = createPhabricatorItsFacade(); + assertThrows(RuntimeException.class, () -> itsFacade.performAction("Foo", "add-project bar")); + + verifyZeroInteractions(conduit); + } + + @Test + public void testPerformActionAddProjectPlain() throws Exception { + when(conduit.maniphestEdit(4711, null, "bar", null)).thenReturn(new ManiphestEdit()); + + PhabricatorItsFacade itsFacade = createPhabricatorItsFacade(); + itsFacade.performAction("4711", "add-project bar"); + + verify(conduit).maniphestEdit(4711, null, "bar", null); + verifyNoMoreInteractions(conduit); + } + + @Test + public void testPerformActionAddProjectConduitException() throws Exception { + when(conduit.maniphestEdit(4711, null, "bar", null)).thenThrow(new ConduitException()); + + PhabricatorItsFacade itsFacade = createPhabricatorItsFacade(); + assertThrows(IOException.class, () -> itsFacade.performAction("4711", "add-project bar")); + + verify(conduit).maniphestEdit(4711, null, "bar", null); + verifyNoMoreInteractions(conduit); + } + + @Test + public void testPerformActionRemoveProjectPlain() throws Exception { + when(conduit.maniphestEdit(4711, null, null, "bar")).thenReturn(new ManiphestEdit()); + + PhabricatorItsFacade itsFacade = createPhabricatorItsFacade(); + itsFacade.performAction("4711", "remove-project bar"); + + verify(conduit).maniphestEdit(4711, null, null, "bar"); + verifyNoMoreInteractions(conduit); + } + + @Test + public void testPerformActionRemoveProjectConduitException() throws Exception { + when(conduit.maniphestEdit(4711, null, null, "bar")).thenThrow(new ConduitException()); + + PhabricatorItsFacade itsFacade = createPhabricatorItsFacade(); + assertThrows(IOException.class, () -> itsFacade.performAction("4711", "remove-project bar")); + + verify(conduit).maniphestEdit(4711, null, null, "bar"); + verifyNoMoreInteractions(conduit); } private PhabricatorItsFacade createPhabricatorItsFacade() { return injector.getInstance(PhabricatorItsFacade.class); } - private void mockUnconnectablePhabricator() { - expect(serverConfig.getString("its-phabricator", null, "url")).andReturn("<no-url>").anyTimes(); - expect(serverConfig.getString("its-phabricator", null, "token")).andReturn("none").anyTimes(); - } - @Override public void setUp() throws Exception { super.setUp(); + serverConfig = mock(Config.class); + conduitFactory = mock(Conduit.Factory.class); + conduit = mock(Conduit.class); + + when(serverConfig.getString("its-phabricator", null, "url")) + .thenReturn("http://phab.example.org/"); + when(serverConfig.getString("its-phabricator", null, "token")).thenReturn("cli-FOO"); + when(conduitFactory.create("http://phab.example.org/", "cli-FOO")).thenReturn(conduit); + injector = Guice.createInjector(new TestModule()); } private class TestModule extends FactoryModule { @Override protected void configure() { - serverConfig = createMock(Config.class); bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(serverConfig); bind(String.class).annotatedWith(PluginName.class).toInstance("its-phabricator"); + bind(Conduit.Factory.class).toInstance(conduitFactory); } } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/ConduitTest.java b/src/test/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/ConduitTest.java index 9dec435..c5dfad1 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/ConduitTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/ConduitTest.java
@@ -13,404 +13,336 @@ // limitations under the License. package com.googlesource.gerrit.plugins.its.phabricator.conduit; -import static org.easymock.EasyMock.capture; -import static org.easymock.EasyMock.eq; -import static org.easymock.EasyMock.expect; -import static org.easymock.EasyMock.resetToStrict; -import static org.powermock.api.easymock.PowerMock.expectNew; +import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.gson.JsonArray; +import com.google.gson.JsonElement; import com.google.gson.JsonObject; import com.google.gson.JsonPrimitive; import com.googlesource.gerrit.plugins.its.base.testutil.LoggingMockingTestCase; import com.googlesource.gerrit.plugins.its.phabricator.conduit.results.ConduitPing; import com.googlesource.gerrit.plugins.its.phabricator.conduit.results.ManiphestEdit; -import com.googlesource.gerrit.plugins.its.phabricator.conduit.results.ManiphestInfo; -import com.googlesource.gerrit.plugins.its.phabricator.conduit.results.ProjectInfo; -import java.util.Arrays; -import java.util.List; +import com.googlesource.gerrit.plugins.its.phabricator.conduit.results.ManiphestSearch; +import com.googlesource.gerrit.plugins.its.phabricator.conduit.results.ProjectSearch; +import java.util.HashMap; import java.util.Map; -import org.easymock.Capture; -import org.junit.runner.RunWith; -import org.powermock.core.classloader.annotations.PrepareForTest; -import org.powermock.modules.junit4.PowerMockRunner; +import org.junit.Before; +import org.junit.Test; -@RunWith(PowerMockRunner.class) -@PrepareForTest(Conduit.class) public class ConduitTest extends LoggingMockingTestCase { private static final String URL = "urlFoo"; private static final String TOKEN = "tokenFoo"; + private ConduitConnection.Factory conduitConnectionFactory; + private ConduitConnection conduitConnection; - private ConduitConnection connection; + @Override + @Before + public void setUp() throws Exception { + super.setUp(); + conduitConnection = mock(ConduitConnection.class); + conduitConnectionFactory = mock(ConduitConnection.Factory.class); + when(conduitConnectionFactory.create(URL)).thenReturn(conduitConnection); + } + @Test public void testConduitPingPass() throws Exception { - mockConnection(); + JsonElement result = new JsonPrimitive("hostFoo"); + Map<String, Object> params = new HashMap<>(); + when(conduitConnection.call("conduit.ping", params, TOKEN)).thenReturn(result); - resetToStrict(connection); - - Capture<Map<String, Object>> paramsCaptureRelevant = new Capture<>(); - - expect(connection.call(eq("conduit.ping"), capture(paramsCaptureRelevant), eq(TOKEN))) - .andReturn(new JsonPrimitive("foo")) - .once(); - - replayMocks(); - - Conduit conduit = new Conduit(URL, TOKEN); + Conduit conduit = createConduit(); ConduitPing actual = conduit.conduitPing(); - - assertEquals("Hostname does not match", "foo", actual.getHostname()); + assertThat(actual.getHostname()).isEqualTo("hostFoo"); } + @Test public void testConduitPingConnectionFail() throws Exception { - mockConnection(); + ConduitException e = new ConduitException(); - resetToStrict(connection); + Map<String, Object> params = new HashMap<>(); + when(conduitConnection.call("conduit.ping", params, TOKEN)).thenThrow(e); - ConduitException conduitException = new ConduitException(); + Conduit conduit = createConduit(); - Capture<Map<String, Object>> paramsCaptureRelevant = new Capture<>(); - - expect(connection.call(eq("conduit.ping"), capture(paramsCaptureRelevant), eq(TOKEN))) - .andThrow(conduitException) - .once(); - - replayMocks(); - - Conduit conduit = new Conduit(URL, TOKEN); - - try { - conduit.conduitPing(); - fail("no exception got thrown"); - } catch (ConduitException e) { - assertSame(conduitException, e); - } + assertThrows(ConduitException.class, () -> conduit.conduitPing()); } - public void testManiphestInfoPass() throws Exception { - mockConnection(); - - resetToStrict(connection); - - Capture<Map<String, Object>> paramsCaptureRelevant = new Capture<>(); - - JsonObject retRelevant = new JsonObject(); - retRelevant.add("id", new JsonPrimitive(42)); - - expect(connection.call(eq("maniphest.info"), capture(paramsCaptureRelevant), eq(TOKEN))) - .andReturn(retRelevant) - .once(); - - replayMocks(); - - Conduit conduit = new Conduit(URL, TOKEN); - - ManiphestInfo maniphestInfo = conduit.maniphestInfo(42); - - Map<String, Object> paramsRelevant = paramsCaptureRelevant.getValue(); - assertEquals("Task id is not set", 42, paramsRelevant.get("task_id")); - - assertEquals("ManiphestInfo's id does not match", 42, maniphestInfo.getId()); - } - - public void testManiphestInfoFailConnect() throws Exception { - mockConnection(); - - ConduitException conduitException = new ConduitException(); - - Capture<Map<String, Object>> paramsCaptureRelevant = new Capture<>(); - - expect(connection.call(eq("maniphest.info"), capture(paramsCaptureRelevant), eq(TOKEN))) - .andThrow(conduitException) - .once(); - - replayMocks(); - - Conduit conduit = new Conduit(URL, TOKEN); - - try { - conduit.maniphestInfo(42); - fail("no exception got thrown"); - } catch (ConduitException e) { - assertSame(conduitException, e); - } - } - - public void testManiphestInfoFailRelevant() throws Exception { - mockConnection(); - - resetToStrict(connection); - - ConduitException conduitException = new ConduitException(); - - Capture<Map<String, Object>> paramsCaptureRelevant = new Capture<>(); - - expect(connection.call(eq("maniphest.info"), capture(paramsCaptureRelevant), eq(TOKEN))) - .andThrow(conduitException) - .once(); - - replayMocks(); - - Conduit conduit = new Conduit(URL, TOKEN); - - try { - conduit.maniphestInfo(42); - fail("no exception got thrown"); - } catch (ConduitException e) { - assertSame(conduitException, e); - } - - Map<String, Object> paramsRelevant = paramsCaptureRelevant.getValue(); - assertEquals("Task id is not set", 42, paramsRelevant.get("task_id")); - } - - public void testManiphestEditPassComment() throws Exception { - mockConnection(); - - resetToStrict(connection); - - JsonObject retRelevant = new JsonObject(); - retRelevant.add("id", new JsonPrimitive(42)); - - Capture<Map<String, Object>> paramsCaptureRelevant = new Capture<>(); - - expect(connection.call(eq("maniphest.update"), capture(paramsCaptureRelevant), eq(TOKEN))) - .andReturn(retRelevant) - .once(); - - replayMocks(); - - Conduit conduit = new Conduit(URL, TOKEN); - - ManiphestEdit maniphestEdit = conduit.maniphestEdit(42, "foo"); - - Map<String, Object> paramsRelevant = paramsCaptureRelevant.getValue(); - assertEquals("Task id is not set", 42, paramsRelevant.get("id")); - - assertEquals("ManiphestInfo's id does not match", 42, maniphestEdit.getId()); - } - - public void testManiphestEditPassProjects() throws Exception { - mockConnection(); - - resetToStrict(connection); - - JsonObject retRelevant = new JsonObject(); - retRelevant.add("id", new JsonPrimitive(42)); - - Capture<Map<String, Object>> paramsCaptureRelevant = new Capture<>(); - - expect(connection.call(eq("maniphest.edit"), capture(paramsCaptureRelevant), eq(TOKEN))) - .andReturn(retRelevant) - .once(); - - replayMocks(); - - Conduit conduit = new Conduit(URL, TOKEN); - - ManiphestEdit maniphestEdit = - conduit.maniphestEdit(42, Arrays.asList("foo", "bar"), Conduit.ACTION_PROJECT_ADD); - - Map<String, Object> paramsRelevant = paramsCaptureRelevant.getValue(); - assertEquals("Task id is not set", 42, paramsRelevant.get("id")); - assertEquals( - "Task projects are not set", - Arrays.asList("foo", "bar"), - paramsRelevant.get("projectPHIDs")); - - assertEquals("ManiphestEdit's id does not match", 42, maniphestEdit.getId()); - } - - public void testManiphestEditPassCommentAndProjects() throws Exception { - mockConnection(); - - resetToStrict(connection); - - JsonObject retRelevant = new JsonObject(); - retRelevant.add("id", new JsonPrimitive(42)); - - Capture<Map<String, Object>> paramsCaptureRelevant = new Capture<>(); - - expect(connection.call(eq("maniphest.edit"), capture(paramsCaptureRelevant), eq(TOKEN))) - .andReturn(retRelevant) - .once(); - - replayMocks(); - - Conduit conduit = new Conduit(URL, TOKEN); - - ManiphestEdit maniphestEdit = - conduit.maniphestEdit( - 42, "baz", Arrays.asList("foo", "bar"), Conduit.ACTION_PROJECT_REMOVE); - - Map<String, Object> paramsRelevant = paramsCaptureRelevant.getValue(); - assertEquals("Task id is not set", 42, paramsRelevant.get("id")); - assertEquals("Task comment is not set", "baz", paramsRelevant.get("comments")); - assertEquals( - "Task projects are not set", - Arrays.asList("foo", "bar"), - paramsRelevant.get("projectPHIDs")); - - assertEquals("ManiphestUpdate's id does not match", 42, maniphestEdit.getId()); - } - - public void testManiphestEditFailConnect() throws Exception { - mockConnection(); - - ConduitException conduitException = new ConduitException(); - - Capture<Map<String, Object>> paramsCapture = new Capture<>(); - - expect(connection.call(eq("conduit.connect"), capture(paramsCapture), eq(TOKEN))) - .andThrow(conduitException) - .once(); - - replayMocks(); - - Conduit conduit = new Conduit(URL, null); - - try { - conduit.maniphestEdit(42, "foo"); - fail("no exception got thrown"); - } catch (ConduitException e) { - assertSame(conduitException, e); - } - } - - public void testManiphestEditFailRelevant() throws Exception { - mockConnection(); - - resetToStrict(connection); - - ConduitException conduitException = new ConduitException(); - - Capture<Map<String, Object>> paramsCaptureRelevant = new Capture<>(); - - expect(connection.call(eq("maniphest.edit"), capture(paramsCaptureRelevant), eq(TOKEN))) - .andThrow(conduitException) - .once(); - - replayMocks(); - - Conduit conduit = new Conduit(URL, TOKEN); - - try { - conduit.maniphestEdit(42, "foo"); - fail("no exception got thrown"); - } catch (ConduitException e) { - assertSame(conduitException, e); - } - - Map<String, Object> paramsRelevant = paramsCaptureRelevant.getValue(); - assertEquals("Task id is not set", 42, paramsRelevant.get("id")); - } - + @Test public void testConnectionReuse() throws Exception { - mockConnection(); + JsonElement result1 = new JsonPrimitive("hostFoo"); + JsonElement result2 = new JsonPrimitive("hostBar"); + Map<String, Object> params = new HashMap<>(); + when(conduitConnection.call("conduit.ping", params, TOKEN)).thenReturn(result1, result2); - resetToStrict(connection); + Conduit conduit = createConduit(); - JsonObject retRelevant = new JsonObject(); - retRelevant.add("id", new JsonPrimitive(42)); + ConduitPing actual1 = conduit.conduitPing(); + assertThat(actual1.getHostname()).isEqualTo("hostFoo"); + ConduitPing actual2 = conduit.conduitPing(); + assertThat(actual2.getHostname()).isEqualTo("hostBar"); - Capture<Map<String, Object>> paramsCaptureRelevant = new Capture<>(); - - expect(connection.call(eq("maniphest.info"), capture(paramsCaptureRelevant), eq(TOKEN))) - .andReturn(retRelevant) - .once(); - - replayMocks(); - - Conduit conduit = new Conduit(URL, TOKEN); - - ManiphestInfo maniphestInfo = conduit.maniphestInfo(42); - - Map<String, Object> paramsRelevant = paramsCaptureRelevant.getValue(); - assertEquals("Task id is not set", 42, paramsRelevant.get("task_id")); - - assertEquals("ManiphestInfo's id does not match", 42, maniphestInfo.getId()); + verify(conduitConnectionFactory).create(URL); + verifyNoMoreInteractions(conduitConnectionFactory); } - public void testProjectQueryPass() throws Exception { - mockConnection(); + @Test + public void testProjectSearchPass() throws Exception { + Map<String, Object> params = new HashMap<>(); + params.put("constraints", ImmutableMap.of("query", "foo")); - resetToStrict(connection); + JsonArray data = new JsonArray(); + data.add(createProjectJson(2, "foo")); - JsonObject projectInfoJson = new JsonObject(); - projectInfoJson.addProperty("name", "foo"); - projectInfoJson.addProperty("phid", "PHID-PROJ-bar"); + JsonObject result = new JsonObject(); + result.add("data", data); - JsonObject queryDataJson = new JsonObject(); - queryDataJson.add("PHID-PROJ-bar", projectInfoJson); + when(conduitConnection.call("project.search", params, TOKEN)).thenReturn(result); - JsonObject retRelevant = new JsonObject(); - retRelevant.add("data", queryDataJson); + Conduit conduit = createConduit(); - Capture<Map<String, Object>> paramsCaptureRelevant = new Capture<>(); - - expect(connection.call(eq("project.query"), capture(paramsCaptureRelevant), eq(TOKEN))) - .andReturn(retRelevant) - .once(); - - replayMocks(); - - Conduit conduit = new Conduit(URL, TOKEN); - - ProjectInfo projectInfo = conduit.projectQuery("foo"); - - Map<String, Object> paramsRelevant = paramsCaptureRelevant.getValue(); - List<String> expectedNames = Arrays.asList("foo"); - assertEquals("Project name does not match", expectedNames, paramsRelevant.get("names")); - - assertEquals("ProjectInfo's name does not match", "foo", projectInfo.getName()); + ProjectSearch actual = conduit.projectSearch("foo"); + assertThat(actual.getPhid()).isEqualTo("PHID-PROJ-foo"); } - public void testProjectQueryPassMultipleResults() throws Exception { - mockConnection(); + @Test + public void testProjectSearchNotFound() throws Exception { + Map<String, Object> params = new HashMap<>(); + params.put("constraints", ImmutableMap.of("query", "foo")); - resetToStrict(connection); + JsonObject result = new JsonObject(); + result.add("data", new JsonArray()); - JsonObject projectInfoJson1 = new JsonObject(); - projectInfoJson1.addProperty("name", "foo1"); - projectInfoJson1.addProperty("phid", "PHID-PROJ-bar1"); + when(conduitConnection.call("project.search", params, TOKEN)).thenReturn(result); - JsonObject projectInfoJson2 = new JsonObject(); - projectInfoJson2.addProperty("name", "foo2"); - projectInfoJson2.addProperty("phid", "PHID-PROJ-bar2"); + Conduit conduit = createConduit(); - JsonObject projectInfoJson3 = new JsonObject(); - projectInfoJson3.addProperty("name", "foo3"); - projectInfoJson3.addProperty("phid", "PHID-PROJ-bar3"); - - JsonObject queryDataJson = new JsonObject(); - queryDataJson.add("PHID-PROJ-bar1", projectInfoJson1); - queryDataJson.add("PHID-PROJ-bar2", projectInfoJson2); - queryDataJson.add("PHID-PROJ-bar3", projectInfoJson3); - - JsonObject retRelevant = new JsonObject(); - retRelevant.add("data", queryDataJson); - - Capture<Map<String, Object>> paramsCaptureRelevant = new Capture<>(); - - expect(connection.call(eq("project.query"), capture(paramsCaptureRelevant), eq(TOKEN))) - .andReturn(retRelevant) - .once(); - - replayMocks(); - - Conduit conduit = new Conduit(URL, TOKEN); - - ProjectInfo projectInfo = conduit.projectQuery("foo2"); - - Map<String, Object> paramsRelevant = paramsCaptureRelevant.getValue(); - List<String> expectedNames = Arrays.asList("foo2"); - assertEquals("Project name does not match", expectedNames, paramsRelevant.get("names")); - - assertEquals("ProjectInfo's name does not match", "foo2", projectInfo.getName()); + ProjectSearch actual = conduit.projectSearch("foo"); + assertThat(actual).isNull(); } - private void mockConnection() throws Exception { - connection = createMock(ConduitConnection.class); - expectNew(ConduitConnection.class, URL).andReturn(connection).once(); + @Test + public void testManiphestEditNoop() throws Exception { + Conduit conduit = createConduit(); + ManiphestEdit actual = conduit.maniphestEdit(4711, null, null, null); + + verifyZeroInteractions(conduitConnection); + assertThat(actual).isNull(); + } + + @Test + public void testManiphestEditEmpty() throws Exception { + Conduit conduit = createConduit(); + ManiphestEdit actual = conduit.maniphestEdit(4711, "", "", ""); + + verifyZeroInteractions(conduitConnection); + assertThat(actual).isNull(); + } + + @Test + public void testManiphestEditAddComment() throws Exception { + Map<String, Object> transaction = new HashMap<>(); + transaction.put("type", "comment"); + transaction.put("value", "foo"); + + Map<String, Object> params = new HashMap<>(); + params.put("objectIdentifier", 4711); + params.put("transactions", ImmutableList.of(transaction)); + + JsonArray data = new JsonArray(); + data.add(createProjectJson(2, "foo")); + + JsonObject response = createEditResponse(1); + when(conduitConnection.call("maniphest.edit", params, TOKEN)).thenReturn(response); + + Conduit conduit = createConduit(); + + ManiphestEdit actual = conduit.maniphestEdit(4711, "foo", null, null); + + assertThat(actual.getObject().getId()).isEqualTo(4712); + assertThat(actual.getObject().getPhid()).isEqualTo("PHID-foo"); + assertThat(actual.getTransactions()).hasSize(1); + assertThat(actual.getTransactions().get(0).getPhid()).isEqualTo("trans@0"); + } + + @Test + public void testManiphestEditAddProject() throws Exception { + Map<String, Object> transaction = new HashMap<>(); + transaction.put("type", "projects.add"); + transaction.put("value", ImmutableList.of("PHID-bar")); + + Map<String, Object> params = new HashMap<>(); + params.put("objectIdentifier", 4711); + params.put("transactions", ImmutableList.of(transaction)); + + JsonArray data = new JsonArray(); + data.add(createProjectJson(2, "foo")); + + JsonObject response = createEditResponse(1); + when(conduitConnection.call("maniphest.edit", params, TOKEN)).thenReturn(response); + + Conduit conduit = spy(createConduit()); + + // shortcut the needed project search + doReturn(new ProjectSearch("PHID-bar", 12)).when(conduit).projectSearch("foo"); + + ManiphestEdit actual = conduit.maniphestEdit(4711, null, "foo", null); + + assertThat(actual.getObject().getId()).isEqualTo(4712); + assertThat(actual.getObject().getPhid()).isEqualTo("PHID-foo"); + assertThat(actual.getTransactions()).hasSize(1); + assertThat(actual.getTransactions().get(0).getPhid()).isEqualTo("trans@0"); + } + + @Test + public void testManiphestEditRemoveProject() throws Exception { + Map<String, Object> transaction = new HashMap<>(); + transaction.put("type", "projects.remove"); + transaction.put("value", ImmutableList.of("PHID-bar")); + + Map<String, Object> params = new HashMap<>(); + params.put("objectIdentifier", 4711); + params.put("transactions", ImmutableList.of(transaction)); + + JsonArray data = new JsonArray(); + data.add(createProjectJson(2, "foo")); + + JsonObject response = createEditResponse(1); + when(conduitConnection.call("maniphest.edit", params, TOKEN)).thenReturn(response); + + Conduit conduit = spy(createConduit()); + + // shortcut the needed project search + doReturn(new ProjectSearch("PHID-bar", 12)).when(conduit).projectSearch("foo"); + + ManiphestEdit actual = conduit.maniphestEdit(4711, null, null, "foo"); + + assertThat(actual.getObject().getId()).isEqualTo(4712); + assertThat(actual.getObject().getPhid()).isEqualTo("PHID-foo"); + assertThat(actual.getTransactions()).hasSize(1); + assertThat(actual.getTransactions().get(0).getPhid()).isEqualTo("trans@0"); + } + + @Test + public void testManiphestEditAllParams() throws Exception { + Map<String, Object> transaction1 = new HashMap<>(); + transaction1.put("type", "comment"); + transaction1.put("value", "foo"); + + Map<String, Object> transaction2 = new HashMap<>(); + transaction2.put("type", "projects.add"); + transaction2.put("value", ImmutableList.of("PHID-bar")); + + Map<String, Object> transaction3 = new HashMap<>(); + transaction3.put("type", "projects.remove"); + transaction3.put("value", ImmutableList.of("PHID-baz")); + + Map<String, Object> params = new HashMap<>(); + params.put("objectIdentifier", 4711); + params.put("transactions", ImmutableList.of(transaction1, transaction2, transaction3)); + + JsonArray data = new JsonArray(); + data.add(createProjectJson(2, "foo")); + + JsonObject response = createEditResponse(3); + when(conduitConnection.call("maniphest.edit", params, TOKEN)).thenReturn(response); + + Conduit conduit = spy(createConduit()); + + // shortcut the needed project searches + doReturn(new ProjectSearch("PHID-bar", 12)).when(conduit).projectSearch("bar"); + doReturn(new ProjectSearch("PHID-baz", 13)).when(conduit).projectSearch("baz"); + + ManiphestEdit actual = conduit.maniphestEdit(4711, "foo", "bar", "baz"); + + assertThat(actual.getObject().getId()).isEqualTo(4712); + assertThat(actual.getObject().getPhid()).isEqualTo("PHID-foo"); + assertThat(actual.getTransactions()).hasSize(3); + assertThat(actual.getTransactions().get(0).getPhid()).isEqualTo("trans@0"); + assertThat(actual.getTransactions().get(1).getPhid()).isEqualTo("trans@1"); + assertThat(actual.getTransactions().get(2).getPhid()).isEqualTo("trans@2"); + } + + @Test + public void testManiphestSearchNotFound() throws Exception { + Map<String, Object> params = new HashMap<>(); + params.put("constraints", ImmutableMap.of("ids", ImmutableList.of(4711))); + + JsonObject result = new JsonObject(); + result.add("data", new JsonArray()); + + when(conduitConnection.call("maniphest.search", params, TOKEN)).thenReturn(result); + Conduit conduit = createConduit(); + + verifyNoMoreInteractions(conduitConnection); + ManiphestSearch actual = conduit.maniphestSearch(4711); + assertThat(actual).isNull(); + } + + @Test + public void testManiphestSearchPass() throws Exception { + Map<String, Object> params = new HashMap<>(); + params.put("constraints", ImmutableMap.of("ids", ImmutableList.of(4711))); + + JsonObject needle = new JsonObject(); + needle.addProperty("id", 23); + + JsonArray data = new JsonArray(); + data.add(needle); + + JsonObject result = new JsonObject(); + result.add("data", data); + + when(conduitConnection.call("maniphest.search", params, TOKEN)).thenReturn(result); + Conduit conduit = createConduit(); + + ManiphestSearch actual = conduit.maniphestSearch(4711); + assertThat(actual.getId()).isEqualTo(23); + } + + private JsonObject createEditResponse(int transactions) { + JsonObject resultObject = new JsonObject(); + resultObject.addProperty("id", 4712); + resultObject.addProperty("phid", "PHID-foo"); + + JsonArray transactionArray = new JsonArray(); + for (int i = 0; i < transactions; i++) { + JsonObject transaction = new JsonObject(); + transaction.addProperty("phid", "trans@" + i); + transactionArray.add(transaction); + } + + JsonObject response = new JsonObject(); + response.add("object", resultObject); + response.add("transactions", transactionArray); + + return response; + } + + private JsonObject createProjectJson(int id, String name) { + JsonObject fields = new JsonObject(); + fields.addProperty("name", name); + fields.addProperty("slug", name); + + JsonObject ret = new JsonObject(); + ret.addProperty("id", id); + ret.addProperty("type", "PROJ"); + ret.addProperty("phid", "PHID-PROJ-" + name); + ret.add("fields", fields); + return ret; + } + + private Conduit createConduit() { + return new Conduit(conduitConnectionFactory, new SearchUtils(), URL, TOKEN); } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/SearchUtilsTest.java b/src/test/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/SearchUtilsTest.java new file mode 100644 index 0000000..676aed5 --- /dev/null +++ b/src/test/java/com/googlesource/gerrit/plugins/its/phabricator/conduit/SearchUtilsTest.java
@@ -0,0 +1,79 @@ +// 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.googlesource.gerrit.plugins.its.phabricator.conduit; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.Lists; +import com.google.gson.JsonArray; +import com.google.gson.JsonObject; +import com.googlesource.gerrit.plugins.its.base.testutil.LoggingMockingTestCase; +import com.googlesource.gerrit.plugins.its.phabricator.conduit.results.PhabObject; +import java.util.List; +import java.util.stream.Stream; +import org.junit.Test; + +public class SearchUtilsTest extends LoggingMockingTestCase { + @Test + public void testStreamEmpty() throws Exception { + JsonObject searchResult = new JsonObject(); + searchResult.add("data", new JsonArray()); + + SearchUtils searchUtils = createSearchUtils(); + Stream<PhabObject> stream = searchUtils.stream(searchResult, PhabObject.class); + List<String> streamAsPhidList = Lists.newArrayList(stream.map(o -> o.getPhid()).iterator()); + assertThat(streamAsPhidList).isEmpty(); + } + + @Test + public void testStreamSingle() throws Exception { + JsonArray data = new JsonArray(); + data.add(createJsonOfPhabObject("PHID1")); + + JsonObject searchResult = new JsonObject(); + searchResult.add("data", data); + + SearchUtils searchUtils = createSearchUtils(); + Stream<PhabObject> stream = searchUtils.stream(searchResult, PhabObject.class); + List<String> streamAsPhidList = Lists.newArrayList(stream.map(o -> o.getPhid()).iterator()); + assertThat(streamAsPhidList).containsExactly("PHID1"); + } + + @Test + public void testStreamMultiple() throws Exception { + JsonArray data = new JsonArray(); + data.add(createJsonOfPhabObject("PHID1")); + data.add(createJsonOfPhabObject("PHID2")); + data.add(createJsonOfPhabObject("PHID3")); + + JsonObject searchResult = new JsonObject(); + searchResult.add("data", data); + + SearchUtils searchUtils = createSearchUtils(); + Stream<PhabObject> stream = searchUtils.stream(searchResult, PhabObject.class); + List<String> streamAsPhidList = Lists.newArrayList(stream.map(o -> o.getPhid()).iterator()); + assertThat(streamAsPhidList).containsExactly("PHID1", "PHID2", "PHID3"); + } + + private SearchUtils createSearchUtils() { + return new SearchUtils(); + } + + private JsonObject createJsonOfPhabObject(String phid) { + JsonObject ret = new JsonObject(); + ret.addProperty("phid", phid); + + return ret; + } +}