Correct Missed Events Playback manager init

It is possible for the Missed Events playback manager to not be correctly initialized once the REST API has been enabled AFTER the connection has been started. The same issue applies if the Gerrit events-log plugin was installed AFTER the Gerrit connection was started. This ensures that the listener is active so that playback can execute when required. [FIXED JENKINS-31439]
jenkinsci · Nov 25, 2015 · 35efed7 · 35efed7
1 parent 8ce1277
commit 35efed7
Show file tree

Hide file tree

Showing 5 changed files with 79 additions and 25 deletions.
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
@@ -410,9 +410,8 @@ public void start() {
                         name, isProjectCreatedEventsSupported());
         projectListUpdater.start();
 
-        if (missedEventsPlaybackManager.isSupported()) {
-            addListener((GerritEventListener)missedEventsPlaybackManager);
-        }
+        missedEventsPlaybackManager.checkIfEventsLogPluginSupported();
+        addListener((GerritEventListener)missedEventsPlaybackManager);
 
         logger.info(name + " started");
         started = true;
@@ -447,6 +446,7 @@ public void stop() {
 
         if (missedEventsPlaybackManager != null) {
             missedEventsPlaybackManager.shutdown();
+            missedEventsPlaybackManager = null;
         }
 
         if (gerritConnection != null) {
@@ -516,10 +516,10 @@ public synchronized void startConnection() {
                 gerritConnection.setHandler(gerritEventManager);
                 gerritConnection.addListener(gerritConnectionListener);
                 gerritConnection.addListener(projectListUpdater);
-                if (missedEventsPlaybackManager == null) {
-                    missedEventsPlaybackManager = new GerritMissedEventsPlaybackManager(name);
-                }
+
+                missedEventsPlaybackManager.checkIfEventsLogPluginSupported();
                 gerritConnection.addListener(missedEventsPlaybackManager);
+
                 gerritConnection.start();
             } else {
                 logger.warn("Already started!");
@@ -870,7 +870,12 @@ public void doConfigSubmit(StaplerRequest req, StaplerResponse rsp) throws Servl
 
         if (!started) {
             this.start();
+        } else {
+            if (missedEventsPlaybackManager != null) {
+                missedEventsPlaybackManager.checkIfEventsLogPluginSupported();
+            }
         }
+
         rsp.sendRedirect("../..");
     }
 

diff --git a/...onyericsson/hudson/plugins/gerrit/trigger/playback/GerritMissedEventsPlaybackManager.java b/...onyericsson/hudson/plugins/gerrit/trigger/playback/GerritMissedEventsPlaybackManager.java
@@ -34,7 +34,6 @@
 import com.sonymobile.tools.gerrit.gerritevents.GerritJsonEventFactory;
 import com.sonymobile.tools.gerrit.gerritevents.dto.GerritEvent;
 import com.sonymobile.tools.gerrit.gerritevents.dto.attr.Provider;
-import com.sonymobile.tools.gerrit.gerritevents.dto.events.ChangeBasedEvent;
 import com.sonymobile.tools.gerrit.gerritevents.dto.events.GerritTriggeredEvent;
 
 import hudson.Util;
@@ -102,17 +101,17 @@ public class GerritMissedEventsPlaybackManager implements ConnectionListener, Ge
     private boolean playBackComplete = false;
 
     /**
-     * @param name Gerrit Server Name
+     * @param name Gerrit Server Name.
      */
     public GerritMissedEventsPlaybackManager(String name) {
         this.serverName = name;
         checkIfEventsLogPluginSupported();
     }
 
     /**
-     * method to verify if plugin is supported
+     * method to verify if plugin is supported.
      */
-    private void checkIfEventsLogPluginSupported() {
+    public void checkIfEventsLogPluginSupported() {
         GerritServer server = PluginImpl.getServer_(serverName);
         if (server != null && server.getConfig() != null) {
             isSupported = GerritPluginChecker.isPluginEnabled(
@@ -323,11 +322,11 @@ private List<GerritTriggeredEvent> createEventsFromString(String eventsString) {
                 continue;
             }
             GerritEvent evt = GerritJsonEventFactory.getEvent(jsonObject);
-            if (evt instanceof ChangeBasedEvent) {
+            if (evt instanceof GerritTriggeredEvent) {
                 Provider provider = new Provider();
                 provider.setName(serverName);
                 ((GerritTriggeredEvent)evt).setProvider(provider);
-                events.add((ChangeBasedEvent)evt);
+                events.add((GerritTriggeredEvent)evt);
             }
         }
         scanner.close();

diff --git a/...sonyericsson/hudson/plugins/gerrit/trigger/playback/GerritMissedEventsFunctionalTest.java b/...sonyericsson/hudson/plugins/gerrit/trigger/playback/GerritMissedEventsFunctionalTest.java
@@ -83,8 +83,6 @@ public class GerritMissedEventsFunctionalTest {
     private static final int HTTPOK = 200;
     private static final int SLEEPTIME = 1000;
 
-    private final String gerritServerName = "testServer";
-    private final String projectName = "testProject";
     private final int port = 29418;
 
     private SshdServerMock server;
@@ -127,12 +125,15 @@ public void tearDown() throws Exception {
 
     /**
      * Test the scenario whereby connection is restarted and events are missed
-     * but replayed.
+     * but replayed. This simulates a restart of Jenkins.
      * @throws Exception Error creating job.
      */
     @Test
     public void testRestartWithMissedEvents() throws Exception {
-        GerritServer gerritServer = new GerritServer(gerritServerName);
+        GerritServer gerritServer = new GerritServer("ZZZZZ");
+        PluginImpl.getInstance().addServer(gerritServer);
+        gerritServer.start();
+
         Config config = (Config)gerritServer.getConfig();
         config.setUseRestApi(true);
         config.setGerritHttpUserName("scott");
@@ -143,10 +144,59 @@ public void testRestartWithMissedEvents() throws Exception {
         config.setGerritAuthKeyFile(sshKey.getPrivateKey());
         gerritServer.setConfig(config);
 
+        gerritServer.startConnection();
+
+        while (!gerritServer.isConnected()) {
+            Thread.sleep(SLEEPTIME);
+        }
+
+        restartWithMissedEvents(gerritServer, "Test ZZZZZ");
+
+    }
+
+    /**
+     * Test the scenario whereby connection is restarted and events are missed
+     * but replayed. This simulates when REST API is enabled and connection is restarted.
+     * @throws Exception Error creating job.
+     */
+    @Test
+    public void testRestartWithRESTApiChangeMissedEvents() throws Exception {
+        GerritServer gerritServer = new GerritServer("ABCDEF");
         PluginImpl.getInstance().addServer(gerritServer);
         gerritServer.start();
+
+        Config config = (Config)gerritServer.getConfig();
+        config.setGerritFrontEndURL("http://localhost:8089");
+        config.setGerritHostName("localhost");
+        config.setGerritProxy("");
+        config.setGerritAuthKeyFile(sshKey.getPrivateKey());
+
         gerritServer.startConnection();
-        FreeStyleProject project = DuplicatesUtil.createGerritTriggeredJob(j, projectName, gerritServerName);
+
+        while (!gerritServer.isConnected()) {
+            Thread.sleep(SLEEPTIME);
+        }
+
+        Config config2 = (Config)gerritServer.getConfig();
+        config2.setUseRestApi(true);
+        config2.setGerritHttpUserName("scott");
+        config2.setGerritHttpPassword("scott");
+
+        //simulate a save of config...it calls doConfigSubmit()
+        gerritServer.getMissedEventsPlaybackManager().checkIfEventsLogPluginSupported();
+
+        restartWithMissedEvents(gerritServer, "Test ABCDEF");
+    }
+
+    /**
+     * Helper method to test the scenarios whereby connection is restarted and events are missed
+     * but replayed.
+     * @param gServer configured Gerrit Server.
+     * @param projectName Project name.
+     * @throws Exception Error creating job.
+     */
+    private void restartWithMissedEvents(GerritServer gServer, String projectName) throws Exception {
+        FreeStyleProject project = DuplicatesUtil.createGerritTriggeredJob(j, projectName, gServer.getName());
 
         GerritTriggerApi api = new GerritTriggerApi();
         Handler handler = null;
@@ -156,17 +206,17 @@ public void testRestartWithMissedEvents() throws Exception {
             fail(ex.getMessage());
         }
         assertNotNull(handler);
-        handler.post(Setup.createPatchsetCreated(gerritServerName));
+        handler.post(Setup.createPatchsetCreated(gServer.getName()));
         TestUtils.waitForBuilds(project, 1);
 
-        assertNotNull(gerritServer.getMissedEventsPlaybackManager().getServerTimestamp());
+        assertNotNull(gServer.getMissedEventsPlaybackManager().getServerTimestamp());
         FreeStyleBuild buildOne = project.getLastCompletedBuild();
         assertSame(Result.SUCCESS, buildOne.getResult());
         assertEquals(1, project.getLastCompletedBuild().getNumber());
-        assertSame(gerritServerName, buildOne.getCause(GerritCause.class).getEvent().getProvider().getName());
+        assertSame(gServer.getName(), buildOne.getCause(GerritCause.class).getEvent().getProvider().getName());
 
-        gerritServer.stopConnection();
-        Thread.currentThread().sleep(SLEEPTIME);
+        gServer.stopConnection();
+        Thread.sleep(SLEEPTIME);
 
         String json = "{\"type\":\"patchset-created\",\"change\":{\"project\":\""
                 + projectName
@@ -198,11 +248,9 @@ public void testRestartWithMissedEvents() throws Exception {
                         .withHeader("Content-Type", "text/html")
                         .withBody(json)));
 
-        gerritServer.restartConnection();
+        gServer.restartConnection();
 
         TestUtils.waitForBuilds(project, 2);
 
     }
-
-
 }
diff --git a/...nyericsson/hudson/plugins/gerrit/trigger/spec/DuplicateGerritListenersHudsonTestCase.java b/...nyericsson/hudson/plugins/gerrit/trigger/spec/DuplicateGerritListenersHudsonTestCase.java
@@ -180,6 +180,7 @@ private void assertNbrOfGerritEventListeners(GerritServer server) {
                 && server.isProjectCreatedEventsSupported()) {
             nbrOfListeners++; // GerritProjectListUpdater adds 1 listener
         }
+        nbrOfListeners++; // GerritMissedEventsPlaybackManager adds 1 listeners
         assertEquals(nbrOfListeners, gerritEventListeners.size());
     }
 }
diff --git a/...n/plugins/gerrit/trigger/spec/DuplicateGerritListenersPreloadedProjectHudsonTestCase.java b/...n/plugins/gerrit/trigger/spec/DuplicateGerritListenersPreloadedProjectHudsonTestCase.java
@@ -364,6 +364,7 @@ void assertNrOfEventListeners(int extra) {
                 && server.isProjectCreatedEventsSupported()) {
             nbrOfListeners++; // GerritProjectListUpdater adds 1 listener//
         }
+        nbrOfListeners++; // GerritMissedEventsPlaybackManager adds 1 listeners
         assertEquals(nbrOfListeners, gerritEventListeners.size());
     }