Skip to content
Permalink
Browse files
Merge pull request #346 from tekkamanendless/master
[JENKINS-39498] Stop panicking about "eventCreatedOn" and losing my position
  • Loading branch information
rsandell committed Feb 6, 2018
2 parents f6cdb2f + faba911 commit e48c9b239c62d1c29204af528ae8ea55e24c7f13
@@ -152,8 +152,13 @@ public String getServerName() {
public void checkIfEventsLogPluginSupported() {
GerritServer server = PluginImpl.getServer_(serverName);
if (server != null && server.getConfig() != null) {
isSupported = GerritPluginChecker.isPluginEnabled(
server.getConfig(), EVENTS_LOG_PLUGIN_NAME, true);
Boolean newValue = GerritPluginChecker.isPluginEnabled(server.getConfig(), EVENTS_LOG_PLUGIN_NAME, true);
if (newValue == null) {
logger.warn("Could not determine plugin support for " + EVENTS_LOG_PLUGIN_NAME
+ "; leaving status as " + String.valueOf(isSupported));
} else {
isSupported = newValue;
}
}
}

@@ -446,19 +451,16 @@ protected String buildEventsLogURL(IGerritHudsonTriggerConfig config, Date date1
* @return true if was able to persist event.
*/
synchronized boolean persist(GerritTriggeredEvent evt) {

// If there is not timestamp, then ignore this event.
if (evt == null || evt.getEventCreatedOn() == null) {
logger.warn("Event CreatedOn is null...Gerrit Server might not support attribute eventCreatedOn. "
+ "Will NOT persist this event and Missed Events will be disabled!");
isSupported = false;
logger.debug("'eventCreatedOn' is null; skipping event.");
return false;
}

long ts = evt.getEventCreatedOn().getTime();
// If the timestamp is invalid, then ignore this event.
if (ts == 0) {
logger.warn("Event CreatedOn is 0...Gerrit Server does not support attribute eventCreatedOn. "
+ "Will NOT persist this event and Missed Events will be disabled!");
isSupported = false;
logger.debug("'eventCreatedOn' is 0; skipping event.");
return false;
}

@@ -32,7 +32,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.net.HttpURLConnection;

/**
@@ -133,21 +132,22 @@ private static boolean decodeStatus(int statusCode, String pluginName, boolean q
* Query Gerrit to determine if plugin is enabled.
* @param config Gerrit Server Config
* @param pluginName The Gerrit Plugin name.
* @return true if enabled.
* @return true if enabled, false if not, and null if we couldn't tell.
*/
public static boolean isPluginEnabled(IGerritHudsonTriggerConfig config, String pluginName) {
public static Boolean isPluginEnabled(IGerritHudsonTriggerConfig config, String pluginName) {
return isPluginEnabled(config, pluginName, false);
}

/**
* Query Gerrit to determine if plugin is enabled.
*
* In the event that this cannot determine the status, it will throw an Exception.
* @param config Gerrit Server Config
* @param pluginName The Gerrit Plugin name.
* @param quiet Whether we want to log a message.
* @return true if enabled.
* @return true if enabled, false if not, and null if we couldn't tell.
*/
public static boolean isPluginEnabled(IGerritHudsonTriggerConfig config, String pluginName, boolean quiet) {

public static Boolean isPluginEnabled(IGerritHudsonTriggerConfig config, String pluginName, boolean quiet) {
String restUrl = buildURL(config);
if (restUrl == null) {
if (quiet) {
@@ -165,18 +165,17 @@ public static boolean isPluginEnabled(IGerritHudsonTriggerConfig config, String
int statusCode = execute.getStatusLine().getStatusCode();
logger.debug("status code: {}", statusCode);
return decodeStatus(statusCode, pluginName, quiet);
} catch (IOException e) {
} catch (Exception e) {
logger.warn(Messages.PluginHttpConnectionGeneralError(pluginName,
e.getMessage()), e);
return false;
} finally {
if (execute != null) {
try {
execute.close();
} catch (Exception exp) {
logger.trace("Error happened when close http client.", exp);
}
}
return null;
}
}
}
@@ -155,7 +155,6 @@ public void setUp() throws IOException {
PowerMockito.mockStatic(GerritPluginChecker.class);
PowerMockito.when(GerritPluginChecker.isPluginEnabled((IGerritHudsonTriggerConfig)anyObject()
, anyString(), anyBoolean())).thenReturn(true);

}

/**
@@ -335,4 +334,59 @@ public void testHandleGarbageResponse() {

}

/**
* This tests that the initial `isSupported` state is false.
*/
@Test
public void testInitialSupportedState() {
// Option 1a: not supported
PowerMockito.when(GerritPluginChecker.isPluginEnabled((IGerritHudsonTriggerConfig)anyObject()
, anyString(), anyBoolean())).thenReturn(false);

GerritMissedEventsPlaybackManager missingEventsPlaybackManager
= new GerritMissedEventsPlaybackManager("defaultServer");
Assert.assertFalse("isSupported should be false", missingEventsPlaybackManager.isSupported());

// Option 1b: not supported
PowerMockito.when(GerritPluginChecker.isPluginEnabled((IGerritHudsonTriggerConfig)anyObject()
, anyString(), anyBoolean())).thenReturn(null);

missingEventsPlaybackManager
= new GerritMissedEventsPlaybackManager("defaultServer");
Assert.assertFalse("isSupported should be false", missingEventsPlaybackManager.isSupported());

// Option 2: supported
PowerMockito.when(GerritPluginChecker.isPluginEnabled((IGerritHudsonTriggerConfig)anyObject()
, anyString(), anyBoolean())).thenReturn(true);

missingEventsPlaybackManager
= new GerritMissedEventsPlaybackManager("defaultServer");
Assert.assertTrue("isSupported should be true", missingEventsPlaybackManager.isSupported());
}

/**
* This tests that the supported state does not change if GerritPluginChecker
* cannot determine the state successfully.
*/
@Test
public void testStateOnlyChangesWhenValid() {
PowerMockito.when(GerritPluginChecker.isPluginEnabled((IGerritHudsonTriggerConfig)anyObject()
, anyString(), anyBoolean())).thenReturn(false);

GerritMissedEventsPlaybackManager missingEventsPlaybackManager
= new GerritMissedEventsPlaybackManager("defaultServer");
Assert.assertFalse("isSupported should be false", missingEventsPlaybackManager.isSupported());

PowerMockito.when(GerritPluginChecker.isPluginEnabled((IGerritHudsonTriggerConfig)anyObject()
, anyString(), anyBoolean())).thenReturn(true);

missingEventsPlaybackManager.checkIfEventsLogPluginSupported();
Assert.assertTrue("isSupported should be true", missingEventsPlaybackManager.isSupported());

PowerMockito.when(GerritPluginChecker.isPluginEnabled((IGerritHudsonTriggerConfig)anyObject()
, anyString(), anyBoolean())).thenReturn(null);

missingEventsPlaybackManager.checkIfEventsLogPluginSupported();
Assert.assertTrue("isSupported should be true", missingEventsPlaybackManager.isSupported());
}
}

0 comments on commit e48c9b2

Please sign in to comment.