Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #112 from jenkinsci/config-improvements
[JENKINS-33228] Misc global config page improvements
  • Loading branch information
lanwen committed Mar 10, 2016
2 parents 20c159e + 2e31640 commit 44a8781
Show file tree
Hide file tree
Showing 12 changed files with 47 additions and 80 deletions.
Expand Up @@ -171,7 +171,7 @@ public boolean configure(StaplerRequest req, JSONObject json) throws FormExcepti

@Override
public String getDisplayName() {
return "GitHub Plugin Configuration";
return "GitHub";
}

@SuppressWarnings("unused")
Expand Down
Expand Up @@ -19,8 +19,6 @@
import org.jenkinsci.plugins.github.util.misc.NullSafePredicate;
import org.jenkinsci.plugins.plaincredentials.StringCredentials;
import org.jenkinsci.plugins.plaincredentials.impl.StringCredentialsImpl;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.github.GitHub;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.DataBoundSetter;
Expand Down Expand Up @@ -82,11 +80,6 @@ public class GitHubServerConfig extends AbstractDescribableImpl<GitHubServerConf
*/
private int clientCacheSize = DEFAULT_CLIENT_CACHE_SIZE_MB;

/**
* only to set to default apiUrl when uncheck. Can be removed if optional block can nullify value if unchecked
*/
private transient boolean customApiUrl;

/**
* To avoid creation of new one on every login with this config
*/
Expand All @@ -98,28 +91,13 @@ public GitHubServerConfig(String credentialsId) {
}

/**
* {@link #customApiUrl} field should be defined earlier. Because of we get full content of optional block,
* even if it already unchecked. So if we want to return api url to default value - custom value should affect
* Set the API endpoint.
*
* @param apiUrl custom url if GH. Default value will be used in case of custom is unchecked or value is blank
*/
@DataBoundSetter
public void setApiUrl(String apiUrl) {
if (customApiUrl) {
this.apiUrl = defaultIfBlank(apiUrl, GITHUB_URL);
} else {
this.apiUrl = GITHUB_URL;
}
}

/**
* Should be called before {@link #setApiUrl(String)}
*
* @param customApiUrl true if optional block "Custom GH Api Url" checked in UI
*/
@DataBoundSetter
public void setCustomApiUrl(boolean customApiUrl) {
this.customApiUrl = customApiUrl;
this.apiUrl = defaultIfBlank(apiUrl, GITHUB_URL);
}

/**
Expand All @@ -136,14 +114,6 @@ public String getApiUrl() {
return apiUrl;
}

/**
* @see #isUrlCustom(String)
*/
@Restricted(NoExternalUse.class)
public boolean isCustomApiUrl() {
return isUrlCustom(apiUrl);
}

public boolean isManageHooks() {
return manageHooks;
}
Expand Down Expand Up @@ -267,7 +237,7 @@ public static class DescriptorImpl extends Descriptor<GitHubServerConfig> {

@Override
public String getDisplayName() {
return "GitHub Server Config";
return "GitHub Server";
}

@SuppressWarnings("unused")
Expand All @@ -287,13 +257,11 @@ ACL.SYSTEM, fromUri(defaultIfBlank(apiUrl, GITHUB_URL)).build())
@SuppressWarnings("unused")
public FormValidation doVerifyCredentials(
@QueryParameter String apiUrl,
@QueryParameter String credentialsId,
@QueryParameter Integer clientCacheSize) throws IOException {
@QueryParameter String credentialsId) throws IOException {

GitHubServerConfig config = new GitHubServerConfig(credentialsId);
config.setCustomApiUrl(isUrlCustom(apiUrl));
config.setApiUrl(apiUrl);
config.setClientCacheSize(clientCacheSize);
config.setClientCacheSize(0);
GitHub gitHub = new GitHubLoginFunction().apply(config);

try {
Expand Down
Expand Up @@ -16,7 +16,6 @@
import java.io.IOException;

import static org.apache.commons.collections.CollectionUtils.isNotEmpty;
import static org.jenkinsci.plugins.github.config.GitHubServerConfig.isUrlCustom;
import static org.jenkinsci.plugins.github.util.FluentIterableWrapper.from;

/**
Expand Down Expand Up @@ -83,7 +82,6 @@ public GitHubServerConfig apply(Credential input) {
);

GitHubServerConfig gitHubServerConfig = new GitHubServerConfig(credentials.getId());
gitHubServerConfig.setCustomApiUrl(isUrlCustom(input.getApiUrl()));
gitHubServerConfig.setApiUrl(input.getApiUrl());

return gitHubServerConfig;
Expand Down
Expand Up @@ -5,15 +5,13 @@ import com.cloudbees.jenkins.GitHubPushTrigger
def f = namespace(lib.FormTagLib);

f.section(title: descriptor.displayName) {
f.entry(title: _("Servers configs with credentials to manage GitHub integrations"),
description: _("List of GitHub Servers to manage hooks, set commit statuses etc."),
f.entry(title: _("GitHub Servers"),
help: descriptor.getHelpFile()) {

f.repeatableHeteroProperty(
field: "configs",
hasHeader: "true",
addCaption: _("Add GitHub Server Config"),
deleteCaption: _("Delete GitHub Server Config"))
addCaption: _("Add GitHub Server"))
}

f.advanced() {
Expand Down
Expand Up @@ -6,35 +6,30 @@ def f = namespace(lib.FormTagLib);
def c = namespace(lib.CredentialsTagLib)


f.entry(title: _("Manage hooks"), field: "manageHooks") {
f.checkbox(default: true)
f.entry(title: _("API URL"), field: "apiUrl") {
f.textbox(default: GitHubServerConfig.GITHUB_URL)
}

f.entry(title: _("Credentials"), field: "credentialsId") {
c.select()
}

f.optionalBlock(title: _("Custom GitHub API URL"),
inline: true,
field: "customApiUrl",
checked: instance?.customApiUrl) {
f.entry(title: _("GitHub API URL"), field: "apiUrl") {
f.textbox(default: GitHubServerConfig.GITHUB_URL)
}
f.block() {
f.validateButton(
title: _("Test connection"),
progress: _("Testing..."),
method: "verifyCredentials",
with: "apiUrl,credentialsId"
)
}


f.entry(title: _("Manage hooks"), field: "manageHooks") {
f.checkbox(default: true)
}

f.advanced() {
f.entry(title: _("GitHub client cache size (MB)"), field: "clientCacheSize") {
f.textbox(default: GitHubServerConfig.DEFAULT_CLIENT_CACHE_SIZE_MB)
}
}

f.block() {
f.validateButton(
title: _("Verify credentials"),
progress: _("Verifying..."),
method: "verifyCredentials",
with: "apiUrl,credentialsId,clientCacheSize"
)
}

@@ -0,0 +1,9 @@
<div>
API endpoint of a GitHub server.

To use public github.com, leave this field
to the default value of <code>https://api.github.com</code>.

Otherwise if you use GitHub Enterprise, specify its API endpoint here
(e.g., <code>https://ghe.acme.com/api/v3/</code>).
</div>
@@ -1,7 +1,12 @@
<div>
Cache size in MB used by GitHub client. This can speed up fetching data form GH and reduce rate limits consuming.<br/>
GH + okHttp do all work for results reliability
(<a href="https://developer.github.com/v3/#conditional-requests">Conditional-requests in GitHub documentation</a>)<br/>

Set <b>0</b> to disable this feature
<p>
Jenkins will use this much space, measured in megabytes,
in <tt>$JENKINS_HOME</tt> to cache data retrieved from GitHub API calls.
A cache will help improve the performance by avoiding unnecessary data transfer, and by doing so it also
makes it less likely to hit <a href="https://developer.github.com/v3/#rate-limiting">API rate limit</a>
(by the use of <a href="https://developer.github.com/v3/#conditional-requests">conditional GET</a> calls.)
</p>
<p>
In an unlikely event that cache is causing a problem, set this to <b>0</b> to disable cache altogether.
</p>
</div>

This file was deleted.

Expand Up @@ -59,7 +59,6 @@ public void shouldMatchDefaultConfigWithGHDefaultHost() throws Exception {
@Test
public void shouldNotMatchNonDefaultConfigWithGHDefaultHost() throws Exception {
GitHubServerConfig input = new GitHubServerConfig("");
input.setCustomApiUrl(true);
input.setApiUrl(CUSTOM_GH_SERVER);
assertThat(withHost(DEFAULT_GH_API_HOST).apply(input), is(false));
}
Expand Down
Expand Up @@ -6,6 +6,7 @@
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.JenkinsRule;
import org.kohsuke.github.GitHub;

import java.io.IOException;
import java.nio.file.DirectoryStream;
Expand Down Expand Up @@ -73,7 +74,6 @@ public void shouldRemoveOnlyNotActiveCachedDirAfterClean() throws Exception {
makeCachedRequestWithCredsId(CHANGED_CREDS_ID);

GitHubServerConfig config = new GitHubServerConfig(CHANGED_CREDS_ID);
config.setCustomApiUrl(true);
config.setApiUrl(github.serverConfig().getApiUrl());
config.setClientCacheSize(1);

Expand All @@ -87,7 +87,6 @@ public void shouldRemoveCacheWhichNotEnabled() throws Exception {
makeCachedRequestWithCredsId(CHANGED_CREDS_ID);

GitHubServerConfig config = new GitHubServerConfig(CHANGED_CREDS_ID);
config.setCustomApiUrl(true);
config.setApiUrl(github.serverConfig().getApiUrl());
config.setClientCacheSize(0);

Expand All @@ -103,7 +102,10 @@ private void it(String comment, int count) throws IOException {
}

private void makeCachedRequestWithCredsId(String credsId) throws IOException {
jRule.getInstance().getDescriptorByType(GitHubServerConfig.DescriptorImpl.class)
.doVerifyCredentials(github.serverConfig().getApiUrl(), credsId, 1);
GitHubServerConfig config = new GitHubServerConfig(credsId);
config.setApiUrl(github.serverConfig().getApiUrl());
config.setClientCacheSize(1);
GitHub gitHub = GitHubServerConfig.loginToGithub().apply(config);
gitHub.getMyself();
}
}
Expand Up @@ -49,7 +49,6 @@ public void shouldPointToSameCacheForOneConfig() throws Exception {
@Test
public void shouldPointToDifferentCachesOnChangedApiPath() throws Exception {
GitHubServerConfig config = new GitHubServerConfig(CREDENTIALS_ID);
config.setCustomApiUrl(true);
config.setApiUrl(CUSTOM_API_URL);

GitHubServerConfig config2 = new GitHubServerConfig(CREDENTIALS_ID);
Expand Down
Expand Up @@ -63,7 +63,6 @@ public WireMockRule service() {
*/
public GitHubServerConfig serverConfig() {
GitHubServerConfig conf = new GitHubServerConfig("creds");
conf.setCustomApiUrl(true);
conf.setApiUrl("http://localhost:" + service().port());
return conf;
}
Expand Down

0 comments on commit 44a8781

Please sign in to comment.