Skip to content
Permalink
Browse files
Fix problem where approvals were not displayed in Query and Trigger G…
…errit Patches window for patchsets which already has Verified or Code-Review approval.

Approvals are displayed only for latest patchset.
Decrease log verbosity for GerritProjectListUpdater.

[FIXED JENKINS-31894]
  • Loading branch information
engycz committed Dec 9, 2015
1 parent 986e00f commit 957082c4409a0a4b362e4fcb895a28ff0df8333f
@@ -238,8 +238,20 @@ public String getJsUrl(String jsName) {
* @param res the patch set.
* @return the highest and lowest code review vote for the patch set.
*/
@Deprecated
public HighLow getCodeReview(JSONObject res) {
return Approval.CODE_REVIEW.getApprovals(res);
return getCodeReview(res, 0);
}

/**
* Finds the highest and lowest code review vote for the provided patch set.
*
* @param res the patch set.
* @param patchSetNumber the patch set number.
* @return the highest and lowest code review vote for the patch set.
*/
public HighLow getCodeReview(JSONObject res, int patchSetNumber) {
return Approval.CODE_REVIEW.getApprovals(res, patchSetNumber);
}

/**
@@ -248,8 +260,20 @@ public HighLow getCodeReview(JSONObject res) {
* @param res the patch-set.
* @return the highest and lowest verified vote.
*/
@Deprecated
public HighLow getVerified(JSONObject res) {
return Approval.VERIFIED.getApprovals(res);
return getVerified(res, 0);
}

/**
* Finds the lowest and highest verified vote for the provided patch set.
*
* @param res the patch-set.
* @param patchSetNumber the patch set number.
* @return the highest and lowest verified vote.
*/
public HighLow getVerified(JSONObject res, int patchSetNumber) {
return Approval.VERIFIED.getApprovals(res, patchSetNumber);
}

/**
@@ -316,15 +340,14 @@ public void doGerritSearch(@QueryParameter("queryString") final String queryStri
JSONArray jsonArray = new JSONArray();
jsonArray.add(j.getJSONObject("currentPatchSet"));
j.put("patchSets", jsonArray);
j.remove("currentPatchSet");
}
}
}
session.setAttribute(SESSION_RESULT, json);
//TODO Implement some smart default selection.
//That can notice that a specific revision is searched or that there is only one result etc.
} catch (GerritQueryException gqe) {
logger.debug("Bad query. ", gqe);
logger.debug("Bad query {}", gqe);
session.setAttribute(SESSION_SEARCH_ERROR, gqe);
} catch (Exception ex) {
logger.warn("Could not query Gerrit for [" + queryString + "]", ex);
@@ -673,26 +696,40 @@ public static enum Approval {
* @param res the change.
* @return the highest and lowest value. Or 0,0 if there are no values.
*/
@Deprecated
public HighLow getApprovals(JSONObject res) {
return getApprovals(res, 0);
}

/**
* Finds the highest and lowest approval value of the approval's type for the specified change.
*
* @param res the change.
* @param patchSetNumber the patch set number.
* @return the highest and lowest value. Or 0,0 if there are no values.
*/
public HighLow getApprovals(JSONObject res, int patchSetNumber) {
logger.trace("Get Approval: {} {}", type, res);
int highValue = Integer.MIN_VALUE;
int lowValue = Integer.MAX_VALUE;
if (res.has("currentPatchSet")) {
logger.trace("Has currentPatchSet");
JSONObject patchSet = res.getJSONObject("currentPatchSet");
if (patchSet.has("approvals")) {
JSONArray approvals = patchSet.getJSONArray("approvals");
logger.trace("Approvals: ", approvals);
for (Object o : approvals) {
JSONObject ap = (JSONObject)o;
if (type.equalsIgnoreCase(ap.optString("type"))) {
logger.trace("A {}", type);
try {
int approval = Integer.parseInt(ap.getString("value"));
highValue = Math.max(highValue, approval);
lowValue = Math.min(lowValue, approval);
} catch (NumberFormatException nfe) {
logger.warn("Gerrit is bad at giving me Approval-numbers!", nfe);
if (patchSet.has("number") && patchSet.has("approvals")) {
if (patchSet.getInt("number") == patchSetNumber) {
JSONArray approvals = patchSet.getJSONArray("approvals");
logger.trace("Approvals: {}", approvals);
for (Object o : approvals) {
JSONObject ap = (JSONObject)o;
if (type.equalsIgnoreCase(ap.optString("type"))) {
logger.trace("A {}", type);
try {
int approval = Integer.parseInt(ap.getString("value"));
highValue = Math.max(highValue, approval);
lowValue = Math.min(lowValue, approval);
} catch (NumberFormatException nfe) {
logger.warn("Gerrit is bad at giving me Approval-numbers! {}", nfe);
}
}
}
}
@@ -169,7 +169,7 @@
</td>
<td align="center" valign="middle"
onClick="activateRow('${theId}')">
<j:set var="v" value="${it.getVerified(res)}"/>
<j:set var="v" value="${it.getVerified(res, patch.getInt('number'))}"/>
<j:choose>
<j:when test="${v.low &lt; 0}">
<img src="${rootURL}/plugin/gerrit-trigger/images/x.gif"
@@ -186,7 +186,7 @@
</td>
<td align="center" valign="middle"
onClick="activateRow('${theId}')">
<j:set var="v" value="${it.getCodeReview(res)}"/>
<j:set var="v" value="${it.getCodeReview(res, patch.getInt('number'))}"/>
<j:choose>
<j:when test="${v.low &lt; -1}">
<img src="${rootURL}/plugin/gerrit-trigger/images/x.gif"
@@ -0,0 +1,225 @@
/*
* The MIT License
*
* Copyright 2015 Jiri Engelthaler. All rights reserved.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/

package com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.actions.manual;

import static org.mockito.Mockito.any;

import com.sonyericsson.hudson.plugins.gerrit.trigger.config.Config;
import com.sonyericsson.hudson.plugins.gerrit.trigger.mock.DuplicatesUtil;
import com.sonyericsson.hudson.plugins.gerrit.trigger.GerritServer;
import com.sonyericsson.hudson.plugins.gerrit.trigger.PluginImpl;
import com.sonymobile.tools.gerrit.gerritevents.mock.SshdServerMock;

import net.sf.json.JSONObject;

import org.apache.sshd.SshServer;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.JenkinsRule;

import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.StaplerResponse;

import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;

import java.util.List;

import javax.servlet.http.HttpSession;

import hudson.model.FreeStyleProject;

//CS IGNORE LineLength FOR NEXT 1 LINES. REASON: static import
import static org.junit.Assert.assertEquals;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

//CS IGNORE LineLength FOR NEXT 1 LINES. REASON: static java import.

/**
* @author Jiri Engelthaler &lt;EngyCZ@gmail.com&gt;
*/

public class ManualTriggerActionApprovalTest {

/**
* An instance of Jenkins Rule.
*/
// CS IGNORE VisibilityModifier FOR NEXT 2 LINES. REASON: JenkinsRule.
@Rule
public final JenkinsRule j = new JenkinsRule();

private final String gerritServerName = "testServer";
private final String projectName = "testProject";
private final int port = 29418;

private SshdServerMock server;
private SshServer sshd;
private SshdServerMock.KeyPairFiles sshKey;

/**
* Runs before test method.
*
* @throws Exception throw if so.
*/
@Before
public void setUp() throws Exception {
sshKey = SshdServerMock.generateKeyPair();
System.setProperty(PluginImpl.TEST_SSH_KEYFILE_LOCATION_PROPERTY, sshKey.getPrivateKey().getAbsolutePath());
server = new SshdServerMock();
sshd = SshdServerMock.startServer(port, server);
server.returnCommandFor("gerrit ls-projects", SshdServerMock.EofCommandMock.class);
server.returnCommandFor("gerrit version", SshdServerMock.EofCommandMock.class);
server.returnCommandFor("gerrit query --format=JSON --current-patch-set \"status:open\"",
SshdServerMock.SendQueryLastPatchSet.class);
server.returnCommandFor("gerrit query --format=JSON --patch-sets --current-patch-set \"status:open\"",
SshdServerMock.SendQueryAllPatchSets.class);
}

/**
* Runs after test method.
*
* @throws Exception throw if so.
*/
@After
public void tearDown() throws Exception {
sshd.stop(true);
sshd = null;
}

/**
* Tests {@link ManualTriggerAction#getApprovals(net.sf.json.JSONObject, int)}.
* With an last patchset
* @throws Exception if so.
*/
@Test
public void testDoGerritSearchLastPatchSet() throws Exception {
GerritServer gerritServer = new GerritServer(gerritServerName);
PluginImpl.getInstance().addServer(gerritServer);
gerritServer.start();

Config config = (Config)gerritServer.getConfig();
config.setGerritAuthKeyFile(sshKey.getPrivateKey());
config.setGerritHostName("localhost");
config.setGerritSshPort(port);
gerritServer.startConnection();

FreeStyleProject project = DuplicatesUtil.createGerritTriggeredJob(j, projectName, gerritServerName);

final ManualTriggerAction action = new ManualTriggerAction();
HttpSession session = mock(HttpSession.class);

StaplerRequest request = mock(StaplerRequest.class);
StaplerResponse response = mock(StaplerResponse.class);

when(request.getSession(true)).thenReturn(session);

doAnswer(new Answer() {
/**
* @see org.mockito.stubbing.Answer#answer(org.mockito.invocation.InvocationOnMock)
*/
@Override
public Object answer(InvocationOnMock aInvocation) throws Throwable {
List<JSONObject> json = (List<JSONObject>)aInvocation.getArguments()[1];

ManualTriggerAction.HighLow highLow = action.getCodeReview(json.get(0), 2);
assertEquals(2, highLow.getHigh());
assertEquals(-1, highLow.getLow());

highLow = action.getVerified(json.get(0), 2);
assertEquals(2, highLow.getHigh());
assertEquals(-1, highLow.getLow());

return null;
}

}).when(session).setAttribute(eq("result"), any());

action.doGerritSearch("status:open", gerritServerName, false, request, response);
}

/**
* Tests {@link ManualTriggerAction#getApprovals(net.sf.json.JSONObject, int)}.
* With an all patchsets
* @throws Exception if so.
*/
@Test
public void testDoGerritSearchAllPatchSets() throws Exception {
GerritServer gerritServer = new GerritServer(gerritServerName);
PluginImpl.getInstance().addServer(gerritServer);
gerritServer.start();

Config config = (Config)gerritServer.getConfig();
config.setGerritAuthKeyFile(sshKey.getPrivateKey());
config.setGerritHostName("localhost");
config.setGerritSshPort(port);
gerritServer.startConnection();

FreeStyleProject project = DuplicatesUtil.createGerritTriggeredJob(j, projectName, gerritServerName);

final ManualTriggerAction action = new ManualTriggerAction();
HttpSession session = mock(HttpSession.class);

StaplerRequest request = mock(StaplerRequest.class);
StaplerResponse response = mock(StaplerResponse.class);

when(request.getSession(true)).thenReturn(session);

doAnswer(new Answer() {
/**
* @see org.mockito.stubbing.Answer#answer(org.mockito.invocation.InvocationOnMock)
*/
@Override
public Object answer(InvocationOnMock aInvocation) throws Throwable {
List<JSONObject> json = (List<JSONObject>)aInvocation.getArguments()[1];

ManualTriggerAction.HighLow highLow = action.getCodeReview(json.get(0), 1);
assertEquals(0, highLow.getHigh());
assertEquals(0, highLow.getLow());

highLow = action.getCodeReview(json.get(0), 2);
assertEquals(2, highLow.getHigh());
assertEquals(-1, highLow.getLow());

highLow = action.getVerified(json.get(0), 1);
assertEquals(0, highLow.getHigh());
assertEquals(0, highLow.getLow());

highLow = action.getVerified(json.get(0), 2);
assertEquals(2, highLow.getHigh());
assertEquals(-1, highLow.getLow());

return null;
}

}).when(session).setAttribute(eq("result"), any());

action.doGerritSearch("status:open", gerritServerName, true, request, response);
}
}

0 comments on commit 957082c

Please sign in to comment.