Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "No connection on bootup" option #168

Merged
merged 5 commits into from Aug 12, 2014
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -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();
Expand Down
Expand Up @@ -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);
}
Expand Down
Expand Up @@ -136,7 +136,9 @@ public class GerritServer implements Describable<GerritServer>, Action {
private static final int RESPONSE_INTERVAL_MS = 1000;
private static final int RESPONSE_TIMEOUT_S = 10;
private String name;
@Deprecated
private boolean pseudoMode;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pseudoMode should also be made transient

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When added transient, assertion error happened in deserialize phase. But it seems there was something wrong in my environment. Now works fine.

OK, I will add it.

private boolean noConnectionOnBootup;
private transient boolean started;
private transient boolean timeoutWakeup = false;
private transient String connectionResponse = "";
Expand Down Expand Up @@ -183,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();
}

Expand Down Expand Up @@ -223,6 +226,7 @@ public String getName() {
*
* @return true if so.
*/
@Deprecated
public boolean isPseudoMode() {
return pseudoMode;
}
Expand All @@ -232,10 +236,29 @@ public boolean isPseudoMode() {
*
* @param pseudoMode true if pseudoMode connection.
*/
@Deprecated
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.
*
Expand Down Expand Up @@ -310,12 +333,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);
Expand Down Expand Up @@ -345,12 +362,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 {
Expand Down Expand Up @@ -425,11 +436,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...");
Expand All @@ -450,11 +456,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);
Expand All @@ -472,9 +473,6 @@ public synchronized void stopConnection() {
*/

public synchronized boolean isConnected() {
if (pseudoMode) {
return true;
}
if (gerritConnection != null) {
return gerritConnection.isConnected();
}
Expand All @@ -486,9 +484,6 @@ public synchronized boolean isConnected() {
*
*/
public void restartConnection() {
if (pseudoMode) {
return;
}
stopConnection();
startConnection();
}
Expand Down Expand Up @@ -570,7 +565,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
Expand All @@ -581,16 +575,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"
Expand Down Expand Up @@ -775,12 +765,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;
Expand All @@ -793,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();
Expand Down
Expand Up @@ -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();
Expand Down
Expand Up @@ -21,10 +21,10 @@
value="${it.name}"
checkUrl="'${rootURL}/${serverURL}/nameFreeCheck?value='+escape(this.value)"/>
</f:entry>
<f:entry title="${%Pseudo Mode}"
help="/plugin/gerrit-trigger/help-GerritPseudoMode.html">
<f:checkbox name="pseudoMode"
checked="${it.pseudoMode}"
<f:entry title="${%No Connection On Bootup}"
help="/plugin/gerrit-trigger/help-GerritNoConnectionOnBootup.html">
<f:checkbox name="noConnectionOnBootup"
checked="${it.noConnectionOnBootup}"
default="false"/>
</f:entry>
<f:entry title="${%Hostname}"
Expand Down
Expand Up @@ -8,8 +8,8 @@ Gerrit\ Server=\
Gerrit\u30b5\u30fc\u30d0\u30fc
Name=\
\u540d\u524d
Pseudo\ Mode=\
\u64ec\u4f3c\u30e2\u30fc\u30c9
No\ Connection\ On\ Bootup=\
\u8d77\u52d5\u6642\u306b\u63a5\u7d9a\u3057\u306a\u3044
Hostname=\
\u30db\u30b9\u30c8\u540d
Frontend\ URL=\
Expand Down
1 change: 1 addition & 0 deletions src/main/webapp/help-GerritNoConnectionOnBootup.html
@@ -0,0 +1 @@
Check if this server connects to Gerrit on bootup.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it the other way around?

Ex: "Check if this server should not connect to Gerrit on bootup."

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I missed. will update.

7 changes: 0 additions & 7 deletions src/main/webapp/help-GerritPseudoMode.html

This file was deleted.

Expand Up @@ -316,23 +316,14 @@ private void addNewServerByCopyingConfig(String newServerName, String fromServer
}

/**
* Test triggering job with events from a Gerrit server with Pseudo mode.
* Test not conect to Gerrit on startup.
* @throws Exception Error creating job.
*/
@Test
public void testTriggeringFromGerritServersWithPseudoMode() throws Exception {
public void testConnectOnStartup() throws Exception {
GerritServer gerritServerOne = new GerritServer(gerritServerOneName, true);
PluginImpl.getInstance().addServer(gerritServerOne);
gerritServerOne.start();
FreeStyleProject projectOne = DuplicatesUtil.createGerritTriggeredJob(this, projectOneName, gerritServerOneName);
PluginImpl.getInstance().getHandler().post(Setup.createPatchsetCreated(gerritServerOneName));
RunList<FreeStyleBuild> 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());
assertEquals(true, gerritServerOne.isNoConnectionOnBootup());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you somehow also check that it actually hasn't tried to connect?

}


}