From 7bc9653ad717baf24da7740a71b0b75e4f6ecd03 Mon Sep 17 00:00:00 2001 From: rinrinne Date: Mon, 4 Aug 2014 17:37:50 +0900 Subject: [PATCH 1/5] Deprecate pseudo mode Previously, gerrit handler could not accept any events during disconnected. So pseudo mode was added to be able to get events in any connection status. After implementation of GerritServer, handler can accept events during disconnected without pseudo mode. So this patch removes it. --- .../gerrit/trigger/GerritManagement.java | 10 ++--- .../trigger/GerritProjectListUpdater.java | 2 +- .../plugins/gerrit/trigger/GerritServer.java | 42 ++----------------- .../gerrit/trigger/GerritServer/index.jelly | 6 --- .../trigger/GerritServerHudsonTest.java | 21 ---------- 5 files changed, 7 insertions(+), 74 deletions(-) diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritManagement.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritManagement.java index 20c66829a..4ce79af97 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritManagement.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritManagement.java @@ -199,14 +199,10 @@ public JSONObject getServerStatuses() { for (GerritServer server : PluginImpl.getInstance().getServers()) { String status; - if (server.isPseudoMode()) { - status = "na"; + if (server.isConnected()) { + status = "up"; } else { - if (server.isConnected()) { - status = "up"; - } else { - status = "down"; - } + status = "down"; } obj = new JSONObject(); diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritProjectListUpdater.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritProjectListUpdater.java index 6b8c8806d..0b274727c 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritProjectListUpdater.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritProjectListUpdater.java @@ -78,7 +78,7 @@ private void addThisAsListener() { GerritServer server = PluginImpl.getInstance().getServer(serverName); if (server != null) { server.addListener(this); - connected = server.isConnected() && (!server.isPseudoMode()); + connected = server.isConnected(); } else { logger.error("Could not find the server {}", serverName); } diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer.java index de403de8c..361744ec6 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer.java @@ -136,6 +136,7 @@ public class GerritServer implements Describable, Action { private static final int RESPONSE_INTERVAL_MS = 1000; private static final int RESPONSE_TIMEOUT_S = 10; private String name; + @Deprecated private boolean pseudoMode; private transient boolean started; private transient boolean timeoutWakeup = false; @@ -223,6 +224,7 @@ public String getName() { * * @return true if so. */ + @Deprecated public boolean isPseudoMode() { return pseudoMode; } @@ -232,6 +234,7 @@ public boolean isPseudoMode() { * * @param pseudoMode true if pseudoMode connection. */ + @Deprecated public void setPseudoMode(boolean pseudoMode) { this.pseudoMode = pseudoMode; } @@ -310,12 +313,6 @@ public void start() { config.setCategories(categories); gerritEventManager = PluginImpl.getInstance().getHandler(); - if (pseudoMode) { - logger.info(name + " started (pseudo mode)"); - started = true; - return; - } - initializeConnectionListener(); projectListUpdater = new GerritProjectListUpdater(name); @@ -345,12 +342,6 @@ private void initializeConnectionListener() { public void stop() { logger.info("Stopping GerritServer " + name); - if (pseudoMode) { - logger.info(name + " stopped (pseudo mode)"); - started = false; - return; - } - if (projectListUpdater != null) { projectListUpdater.shutdown(); try { @@ -425,11 +416,6 @@ public GerritConnectionListener getGerritConnectionListener() { * */ public synchronized void startConnection() { - if (pseudoMode) { - gerritEventManager.setIgnoreEMail(name, config.getGerritEMail()); - return; - } - if (!config.hasDefaultValues()) { if (gerritConnection == null) { logger.debug("Starting Gerrit connection..."); @@ -450,11 +436,6 @@ public synchronized void startConnection() { * */ public synchronized void stopConnection() { - if (pseudoMode) { - gerritEventManager.setIgnoreEMail(name, null); - return; - } - if (gerritConnection != null) { gerritConnection.shutdown(true); gerritConnection.removeListener(gerritConnectionListener); @@ -472,9 +453,6 @@ public synchronized void stopConnection() { */ public synchronized boolean isConnected() { - if (pseudoMode) { - return true; - } if (gerritConnection != null) { return gerritConnection.isConnected(); } @@ -486,9 +464,6 @@ public synchronized boolean isConnected() { * */ public void restartConnection() { - if (pseudoMode) { - return; - } stopConnection(); startConnection(); } @@ -570,7 +545,6 @@ public String getDisplayName() { /** * Tests if the provided parameters can connect to Gerrit. - * @param pseudoMode if pseudo mode or not. * @param gerritHostName the hostname * @param gerritSshPort the ssh-port * @param gerritProxy the proxy url @@ -581,16 +555,12 @@ public String getDisplayName() { * {@link FormValidation#error(java.lang.String) } otherwise. */ public FormValidation doTestConnection( - @QueryParameter("pseudoMode") final boolean pseudoMode, @QueryParameter("gerritHostName") final String gerritHostName, @QueryParameter("gerritSshPort") final int gerritSshPort, @QueryParameter("gerritProxy") final String gerritProxy, @QueryParameter("gerritUserName") final String gerritUserName, @QueryParameter("gerritAuthKeyFile") final String gerritAuthKeyFile, @QueryParameter("gerritAuthKeyFilePassword") final String gerritAuthKeyFilePassword) { - if (pseudoMode) { - return FormValidation.ok(Messages.Success()); - } if (logger.isDebugEnabled()) { logger.debug("gerritHostName = {}\n" + "gerritSshPort = {}\n" @@ -775,12 +745,6 @@ public void doConfigSubmit(StaplerRequest req, StaplerResponse rsp) throws Servl logger.debug("submit {}", req.toString()); } JSONObject form = req.getSubmittedForm(); - if (isConnected()) { - pseudoMode = false; - form.put("pseudoMode", false); - } else { - pseudoMode = form.getBoolean("pseudoMode"); - } String newName = form.getString("name"); boolean renamed = false; diff --git a/src/main/resources/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer/index.jelly b/src/main/resources/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer/index.jelly index 3fdf98e01..908f4f277 100644 --- a/src/main/resources/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer/index.jelly +++ b/src/main/resources/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer/index.jelly @@ -21,12 +21,6 @@ value="${it.name}" checkUrl="'${rootURL}/${serverURL}/nameFreeCheck?value='+escape(this.value)"/> - - - buildsOne = DuplicatesUtil.waitForBuilds(projectOne, 1, timeToBuild); - - FreeStyleBuild buildOne = buildsOne.get(0); - assertSame(Result.SUCCESS, buildOne.getResult()); - assertEquals(1, projectOne.getBuilds().size()); - assertSame(gerritServerOneName, buildOne.getCause(GerritCause.class).getEvent().getProvider().getName()); - } - - } From 94c72b9c362ede876ee512934091d1e5c2f3356a Mon Sep 17 00:00:00 2001 From: rinrinne Date: Mon, 4 Aug 2014 19:37:08 +0900 Subject: [PATCH 2/5] Add "No connection on bootup" option in server config This patch adds a feature that connection to server is not established on bootup. Fix for JENKINS-24099 Task-Url: https://issues.jenkins-ci.org/browse/JENKINS-24099 --- .../plugins/gerrit/trigger/GerritServer.java | 27 ++++++++++++++++--- .../hudsontrigger/GerritItemListener.java | 4 ++- .../gerrit/trigger/GerritServer/index.jelly | 6 +++++ .../trigger/GerritServer/index_ja.properties | 4 +-- .../help-GerritNoConnectionOnBootup.html | 1 + src/main/webapp/help-GerritPseudoMode.html | 7 ----- .../trigger/GerritServerHudsonTest.java | 12 +++++++++ 7 files changed, 48 insertions(+), 13 deletions(-) create mode 100644 src/main/webapp/help-GerritNoConnectionOnBootup.html delete mode 100644 src/main/webapp/help-GerritPseudoMode.html diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer.java index 361744ec6..b8111b6ce 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer.java @@ -138,6 +138,7 @@ public class GerritServer implements Describable, Action { private String name; @Deprecated private boolean pseudoMode; + private boolean noConnectionOnBootup; private transient boolean started; private transient boolean timeoutWakeup = false; private transient String connectionResponse = ""; @@ -184,11 +185,12 @@ public GerritServer(String name) { * Constructor. * * @param name the name of the server. - * @param pseudoMode if pseudo mode or not. + * @param noConnectionOnBootup if noConnectionOnBootup or not. */ - public GerritServer(String name, boolean pseudoMode) { + public GerritServer(String name, boolean noConnectionOnBootup) { this.name = name; - this.pseudoMode = pseudoMode; + this.pseudoMode = false; + this.noConnectionOnBootup = noConnectionOnBootup; config = new Config(); } @@ -239,6 +241,24 @@ public void setPseudoMode(boolean pseudoMode) { this.pseudoMode = pseudoMode; } + /** + * If no connection on bootup or not. + * + * @return true if so. + */ + public boolean isNoConnectionOnBootup() { + return noConnectionOnBootup; + } + + /** + * Sets connect on bootup. + * + * @param noConnectionOnBootup true if connect on bootup. + */ + public void setNoConnectionOnBootup(boolean noConnectionOnBootup) { + this.noConnectionOnBootup = noConnectionOnBootup; + } + /** * Gets wakeup is failed by timeout or not. * @@ -757,6 +777,7 @@ public void doConfigSubmit(StaplerRequest req, StaplerResponse rsp) throws Servl rename(newName); renamed = true; } + noConnectionOnBootup = form.getBoolean("noConnectionOnBootup"); config.setValues(form); PluginImpl.getInstance().save(); diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritItemListener.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritItemListener.java index f79ec51ba..e2bc10488 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritItemListener.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritItemListener.java @@ -77,7 +77,9 @@ public void onDeleted(Item item) { public void onLoaded() { if (PluginImpl.getInstance() != null) { for (GerritServer s : PluginImpl.getInstance().getServers()) { - s.startConnection(); + if (!s.isNoConnectionOnBootup()) { + s.startConnection(); + } } } super.onLoaded(); diff --git a/src/main/resources/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer/index.jelly b/src/main/resources/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer/index.jelly index 908f4f277..3180a3234 100644 --- a/src/main/resources/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer/index.jelly +++ b/src/main/resources/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer/index.jelly @@ -21,6 +21,12 @@ value="${it.name}" checkUrl="'${rootURL}/${serverURL}/nameFreeCheck?value='+escape(this.value)"/> + + + Date: Tue, 12 Aug 2014 14:23:29 +0900 Subject: [PATCH 3/5] Add transient modifier to pseudoMode --- .../hudson/plugins/gerrit/trigger/GerritServer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer.java index b8111b6ce..8698cfa65 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer.java @@ -137,7 +137,7 @@ public class GerritServer implements Describable, Action { private static final int RESPONSE_TIMEOUT_S = 10; private String name; @Deprecated - private boolean pseudoMode; + private transient boolean pseudoMode; private boolean noConnectionOnBootup; private transient boolean started; private transient boolean timeoutWakeup = false; From e93ca93f1496212cba06da6db72b40db61594294 Mon Sep 17 00:00:00 2001 From: rinrinne Date: Tue, 12 Aug 2014 14:33:06 +0900 Subject: [PATCH 4/5] Unified wording to 'startup' --- .../plugins/gerrit/trigger/GerritServer.java | 24 +++++++++---------- .../hudsontrigger/GerritItemListener.java | 2 +- .../gerrit/trigger/GerritServer/index.jelly | 8 +++---- .../trigger/GerritServer/index_ja.properties | 2 +- .../help-GerritNoConnectionOnBootup.html | 1 - .../help-GerritNoConnectionOnStartup.html | 1 + .../trigger/GerritServerHudsonTest.java | 2 +- 7 files changed, 20 insertions(+), 20 deletions(-) delete mode 100644 src/main/webapp/help-GerritNoConnectionOnBootup.html create mode 100644 src/main/webapp/help-GerritNoConnectionOnStartup.html diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer.java index 8698cfa65..b7d3b12b5 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer.java @@ -138,7 +138,7 @@ public class GerritServer implements Describable, Action { private String name; @Deprecated private transient boolean pseudoMode; - private boolean noConnectionOnBootup; + private boolean noConnectionOnStartup; private transient boolean started; private transient boolean timeoutWakeup = false; private transient String connectionResponse = ""; @@ -185,12 +185,12 @@ public GerritServer(String name) { * Constructor. * * @param name the name of the server. - * @param noConnectionOnBootup if noConnectionOnBootup or not. + * @param noConnectionOnStartup if noConnectionOnStartup or not. */ - public GerritServer(String name, boolean noConnectionOnBootup) { + public GerritServer(String name, boolean noConnectionOnStartup) { this.name = name; this.pseudoMode = false; - this.noConnectionOnBootup = noConnectionOnBootup; + this.noConnectionOnStartup = noConnectionOnStartup; config = new Config(); } @@ -242,21 +242,21 @@ public void setPseudoMode(boolean pseudoMode) { } /** - * If no connection on bootup or not. + * If no connection on startup or not. * * @return true if so. */ - public boolean isNoConnectionOnBootup() { - return noConnectionOnBootup; + public boolean isNoConnectionOnStartup() { + return noConnectionOnStartup; } /** - * Sets connect on bootup. + * Sets connect on startup. * - * @param noConnectionOnBootup true if connect on bootup. + * @param noConnectionOnStartup true if connect on startup. */ - public void setNoConnectionOnBootup(boolean noConnectionOnBootup) { - this.noConnectionOnBootup = noConnectionOnBootup; + public void setNoConnectionOnStartup(boolean noConnectionOnStartup) { + this.noConnectionOnStartup = noConnectionOnStartup; } /** @@ -777,7 +777,7 @@ public void doConfigSubmit(StaplerRequest req, StaplerResponse rsp) throws Servl rename(newName); renamed = true; } - noConnectionOnBootup = form.getBoolean("noConnectionOnBootup"); + noConnectionOnStartup = form.getBoolean("noConnectionOnStartup"); config.setValues(form); PluginImpl.getInstance().save(); diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritItemListener.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritItemListener.java index e2bc10488..145bd63ee 100644 --- a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritItemListener.java +++ b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritItemListener.java @@ -77,7 +77,7 @@ public void onDeleted(Item item) { public void onLoaded() { if (PluginImpl.getInstance() != null) { for (GerritServer s : PluginImpl.getInstance().getServers()) { - if (!s.isNoConnectionOnBootup()) { + if (!s.isNoConnectionOnStartup()) { s.startConnection(); } } diff --git a/src/main/resources/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer/index.jelly b/src/main/resources/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer/index.jelly index 3180a3234..3065b38bc 100644 --- a/src/main/resources/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer/index.jelly +++ b/src/main/resources/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer/index.jelly @@ -21,10 +21,10 @@ value="${it.name}" checkUrl="'${rootURL}/${serverURL}/nameFreeCheck?value='+escape(this.value)"/> - - + Date: Tue, 12 Aug 2014 15:47:04 +0900 Subject: [PATCH 5/5] Add tests for GerritItemListener#onLoaded --- .../hudsontrigger/GerritItemListenerTest.java | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritItemListenerTest.java diff --git a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritItemListenerTest.java b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritItemListenerTest.java new file mode 100644 index 000000000..235d36626 --- /dev/null +++ b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritItemListenerTest.java @@ -0,0 +1,103 @@ +/* + * The MIT License + * + * Copyright 2014 rinrinne a.k.a. rin_ne 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; + +import static org.powermock.api.mockito.PowerMockito.mock; +import static org.powermock.api.mockito.PowerMockito.mockStatic; +import static org.powermock.api.mockito.PowerMockito.when; +import static org.powermock.api.mockito.PowerMockito.spy; +import static org.powermock.api.mockito.PowerMockito.doNothing; + +import java.util.Arrays; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.jvnet.hudson.test.JenkinsRule; +import org.mockito.Mockito; +import org.powermock.core.classloader.annotations.PowerMockIgnore; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; + +import com.sonyericsson.hudson.plugins.gerrit.trigger.GerritServer; +import com.sonyericsson.hudson.plugins.gerrit.trigger.PluginImpl; + +/** + * Tests for {@link GerritItemListener}. + */ +@RunWith(PowerMockRunner.class) +@PrepareForTest(PluginImpl.class) +@PowerMockIgnore({"javax.crypto.*" }) +public class GerritItemListenerTest { + + /** + * Jenkins rule instance. + */ + // CS IGNORE VisibilityModifier FOR NEXT 3 LINES. REASON: Mocks tests. + @Rule + public JenkinsRule j = new JenkinsRule(); + + private final String gerritServerName = "testServer"; + private GerritServer gerritServer; + + /** + * Setup the mock'ed environment. + */ + @Before + public void setup() { + PluginImpl plugin = mock(PluginImpl.class); + mockStatic(PluginImpl.class); + when(PluginImpl.getInstance()).thenReturn(plugin); + gerritServer = spy(new GerritServer(gerritServerName)); + doNothing().when(gerritServer).startConnection(); + when(plugin.getServers()).thenReturn(Arrays.asList(gerritServer)); + } + + /** + * Tests {@link GerritItemListener#onLoad()} gets connection. + * + * @throws Exception if so. + */ + @Test + public void testOnLoadedWithConnection() throws Exception { + gerritServer.setNoConnectionOnStartup(false); + GerritItemListener listener = new GerritItemListener(); + listener.onLoaded(); + Mockito.verify(gerritServer, Mockito.times(1)).startConnection(); + } + + /** + * Tests {@link GerritItemListener#onLoad()} does not get connection. + * + * @throws Exception if so. + */ + @Test + public void testOnLoadedWithNoConnection() throws Exception { + gerritServer.setNoConnectionOnStartup(true); + GerritItemListener listener = new GerritItemListener(); + listener.onLoaded(); + Mockito.verify(gerritServer, Mockito.times(0)).startConnection(); + } +}