10000 Merge pull request #112 from jenkinsci/config-improvements · github-cloud/github-plugin@44a8781 · GitHub
[go: up one dir, main page]
Skip to content

Commit 44a8781

Browse files
lanwenlanwen
committed
Merge pull request jenkinsci#112 from jenkinsci/config-improvements
[JENKINS-33228] Misc global config page improvements
2 parents 20c159e + 2e31640 commit 44a8781

File tree

12 files changed

+47
-80
lines changed

12 files changed

+47
-80
lines changed

‎src/main/java/org/jenkinsci/plugins/github/config/GitHubPluginConfig.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ public boolean configure(StaplerRequest req, JSONObject json) throws FormExcepti
171171

172172
@Override
173173
public String getDisplayName() {
174-
return "GitHub Plugin Configuration";
174+
return "GitHub";
175175
}
176176

177177
@SuppressWarnings("unused")

‎src/main/java/org/jenkinsci/plugins/github/config/GitHubServerConfig.java

Lines changed: 5 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919
import org.jenkinsci.plugins.github.util.misc.NullSafePredicate;
2020
import org.jenkinsci.plugins.plaincredentials.StringCredentials;
2121
import org.jenkinsci.plugins.plaincredentials.impl.StringCredentialsImpl;
22-
import org.kohsuke.accmod.Restricted;
23-
import org.kohsuke.accmod.restrictions.NoExternalUse;
2422
import org.kohsuke.github.GitHub;
2523
import org.kohsuke.stapler.DataBoundConstructor;
2624
import org.kohsuke.stapler.DataBoundSetter;
@@ -82,11 +80,6 @@ public class GitHubServerConfig extends AbstractDescribableImpl<GitHubServerConf
8280
*/
8381
private int clientCacheSize = DEFAULT_CLIENT_CACHE_SIZE_MB;
8482

85-
/**
86-
* only to set to default apiUrl when uncheck. Can be removed if optional block can nullify value if unchecked
87-
*/
88-
private transient boolean customApiUrl;
89-
9083
/**
9184
* To avoid creation of new one on every login with this config
9285
*/
@@ -98,28 +91,13 @@ public GitHubServerConfig(String credentialsId) {
9891
}
9992

10093
/**
101-
* {@link #customApiUrl} field should be defined earlier. Because of we get full content of optional block,
102-
* even if it already unchecked. So if we want to return api url to default value - custom value should affect
94+
* Set the API endpoint.
10395
*
10496
* @param apiUrl custom url if GH. Default value will be used in case of custom is unchecked or value is blank
10597
*/
10698
@DataBoundSetter
10799
public void setApiUrl(String apiUrl) {
108-
if (customApiUrl) {
109-
this.apiUrl = defaultIfBlank(apiUrl, GITHUB_URL);
110-
} else {
111-
this.apiUrl = GITHUB_URL;
112-
}
113-
}
114-
115-
/**
116-
* Should be called before {@link #setApiUrl(String)}
117-
*
118-
* @param customApiUrl true if optional block "Custom GH Api Url" checked in UI
119-
*/
120-
@DataBoundSetter
121-
public void setCustomApiUrl(boolean customApiUrl) {
122-
this.customApiUrl = customApiUrl;
100+
this.apiUrl = defaultIfBlank(apiUrl, GITHUB_URL);
123101
}
124102

125103
/**
@@ -136,14 +114,6 @@ public String getApiUrl() {
136114
return apiUrl;
137115
}
138116

139-
/**
140-
* @see #isUrlCustom(String)
141-
*/
142-
@Restricted(NoExternalUse.class)
143-
public boolean isCustomApiUrl() {
144-
return isUrlCustom(apiUrl);
145-
}
146-
147117
public boolean isManageHooks() {
148118
return manageHooks;
149119
}
@@ -267,7 +237,7 @@ public static class DescriptorImpl extends Descriptor<GitHubServerConfig> {
267237

268238
@Override
269239
public String getDisplayName() {
270-
return "GitHub Server Config";
240+
return "GitHub Server";
271241
}
272242

273243
@SuppressWarnings("unused")
@@ -287,13 +257,11 @@ ACL.SYSTEM, fromUri(defaultIfBlank(apiUrl, GITHUB_URL)).build())
287257
@SuppressWarnings("unused")
288258
public FormValidation doVerifyCredentials(
289259
@QueryParameter String apiUrl,
290-
@QueryParameter String credentialsId,
291-
@QueryParameter Integer clientCacheSize) throws IOException {
260+
@QueryParameter String credentialsId) throws IOException {
292261

293262
GitHubServerConfig config = new GitHubServerConfig(credentialsId);
294-
config.setCustomApiUrl(isUrlCustom(apiUrl));
295263
config.setApiUrl(apiUrl);
296-
config.setClientCacheSize(clientCacheSize);
264+
config.setClientCacheSize(0);
297265
GitHub gitHub = new GitHubLoginFunction().apply(config);
298266

299267
try {

‎src/main/java/org/jenkinsci/plugins/github/migration/Migrator.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import java.io.IOException;
1717

1818
import static org.apache.commons.collections.CollectionUtils.isNotEmpty;
19-
import static org.jenkinsci.plugins.github.config.GitHubServerConfig.isUrlCustom;
2019
import static org.jenkinsci.plugins.github.util.FluentIterableWrapper.from;
2120

2221
/**
@@ -83,7 +82,6 @@ public GitHubServerConfig apply(Credential input) {
8382
);
8483

8584
GitHubServerConfig gitHubServerConfig = new GitHubServerConfig(credentials.getId());
86-
gitHubServerConfig.setCustomApiUrl(isUrlCustom(input.getApiUrl()));
8785
gitHubServerConfig.setApiUrl(input.getApiUrl());
8886

8987
return gitHubServerConfig;

‎src/main/resources/org/jenkinsci/plugins/github/config/GitHubPluginConfig/config.groovy

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,13 @@ import com.cloudbees.jenkins.GitHubPushTrigger
55
def f = namespace(lib.FormTagLib);
66

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

1211
f.repeatableHeteroProperty(
1312
field: "configs",
1413
hasHeader: "true",
15-
addCaption: _("Add GitHub Server Config"),
16-
deleteCaption: _("Delete GitHub Server Config"))
14+
addCaption: _("Add GitHub Server"))
1715
}
1816

1917
f.advanced() {

‎src/main/resources/org/jenkinsci/plugins/github/config/GitHubServerConfig/config.groovy

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,35 +6,30 @@ def f = namespace(lib.FormTagLib);
66
def c = namespace(lib.CredentialsTagLib)
77

88

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

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

17-
f.optionalBlock(title: _("Custom GitHub API URL"),
18-
inline: true,
19-
field: "customApiUrl",
20-
checked: instance?.customApiUrl) {
21-
f.entry(title: _(" 106A1 ;GitHub API URL"), field: "apiUrl") {
22-
f.textbox(default: GitHubServerConfig.GITHUB_URL)
23-
}
17+
f.block() {
18+
f.validateButton(
19+
title: _("Test connection"),
20+
progress: _("Testing..."),
21+
method: "verifyCredentials",
22+
with: "apiUrl,credentialsId"
23+
)
24+
}
25+
26+
27+
f.entry(title: _("Manage hooks"), field: "manageHooks") {
28+
f.checkbox(default: true)
2429
}
2530

2631
f.advanced() {
2732
f.entry(title: _("GitHub client cache size (MB)"), field: "clientCacheSize") {
2833
f.textbox(default: GitHubServerConfig.DEFAULT_CLIENT_CACHE_SIZE_MB)
2934
}
3035
}
31-
32-
f.block() {
33-
f.validateButton(
34-
title: _("Verify credentials"),
35-
progress: _("Verifying..."),
36-
method: "verifyCredentials",
37-
with: "apiUrl,credentialsId,clientCacheSize"
38-
)
39-
}
40-

‎src/main/resources/org/jenkinsci/plugins/github/config/GitHubServerConfig/help-apiUrl.html

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<div>
2+
API endpoint of a GitHub server.
3+
4+
To use public github.com, leave this field
5+
to the default value of <code>https://api.github.com</code>.
6+
7+
Otherwise if you use GitHub Enterprise, specify its API endpoint here
8+
(e.g., <code>https://ghe.acme.com/api/v3/</code>).
9+
</div>

‎src/main/resources/org/jenkinsci/plugins/github/config/GitHubServerConfig/help-clientCacheSize.html

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
<div>
2-
Cache size in MB used by GitHub client. This can speed up fetching data form GH and reduce rate limits consuming.<br/>
3-
GH + okHttp do all work for results reliability
4-
(<a href="https://developer.github.com/v3/#conditional-requests">Conditional-requests in GitHub documentation</a>)<br/>
5-
6-
Set <b>0</b> to disable this feature
2+
<p>
3+
Jenkins will use this much space, measured in megabytes,
4+
in <tt>$JENKINS_HOME</tt> to cache data retrieved from GitHub API calls.
5+
A cache will help improve the performance by avoiding unnecessary data transfer, and by doing so it also
6+
makes it less likely to hit <a href="https://developer.github.com/v3/#rate-limiting">API rate limit</a>
7+
(by the use of <a href="https://developer.github.com/v3/#conditional-requests">conditional GET</a> calls.)
8+
</p>
9+
<p>
10+
In an unlikely event that cache is causing a problem, set this to <b>0</b> to disable cache altogether.
11+
</p>
712
</div>

‎src/main/resources/org/jenkinsci/plugins/github/config/GitHubServerConfig/help-customApiUrl.html

Lines changed: 0 additions & 5 deletions
This file was deleted.

‎src/test/java/org/jenkinsci/plugins/github/config/GitHubServerConfigTest.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ public void shouldMatchDefaultConfigWithGHDefaultHost() throws Exception {
5959
@Test
6060
public void shouldNotMatchNonDefaultConfigWithGHDefaultHost() throws Exception {
6161
GitHubServerConfig input = new GitHubServerConfig("");
62-
input.setCustomApiUrl(true);
6362
input.setApiUrl(CUSTOM_GH_SERVER);
6463
assertThat(withHost(DEFAULT_GH_API_HOST).apply(input), is(false));
6564
}

‎src/test/java/org/jenkinsci/plugins/github/internal/GitHubClientCacheCleanupTest.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import org.junit.Rule;
77
import org.junit.Test;
88
import org.jvnet.hudson.test.JenkinsRule;
9+
import org.kohsuke.github.GitHub;
910

1011
import java.io.IOException;
1112
import java.nio.file.DirectoryStream;
@@ -73,7 +74,6 @@ public void shouldRemoveOnlyNotActiveCachedDirAfterClean() throws Exception {
7374
makeCachedRequestWithCredsId(CHANGED_CREDS_ID);
7475

7576
GitHubServerConfig config = new GitHubServerConfig(CHANGED_CREDS_ID);
76-
config.setCustomApiUrl(true);
7777
config.setApiUrl(github.serverConfig().getApiUrl());
7878
config.setClientCacheSize(1);
7979

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

8989
GitHubServerConfig config = new GitHubServerConfig(CHANGED_CREDS_ID);
90-
config.setCustomApiUrl(true);
9190
config.setApiUrl(github.serverConfig().getApiUrl());
9291
config.setClientCacheSize(0);
9392

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

105104
private void makeCachedRequestWithCredsId(String credsId) throws IOException {
106-
jRule.getInstance().getDescriptorByType(GitHubServerConfig.DescriptorImpl.class)
107-
.doVerifyCredentials(github.serverConfig().getApiUrl(), credsId, 1);
105+
GitHubServerConfig config = new GitHubServerConfig(credsId);
106+
config.setApiUrl(github.serverConfig().getApiUrl());
107+
config.setClientCacheSize(1);
108+
GitHub gitHub = GitHubServerConfig.loginToGithub().apply(config);
109+
gitHub.getMyself();
108110
}
109111
}

‎src/test/java/org/jenkinsci/plugins/github/internal/GitHubClientCacheOpsTest.java

Lines changed: 0 additions & 1 deletion
8CB6
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ public void shouldPointToSameCacheForOneConfig() throws Exception {
4949
@Test
5050
public void shouldPointToDifferentCachesOnChangedApiPath() throws Exception {
5151
GitHubServerConfig config = new GitHubServerConfig(CREDENTIALS_ID);
52-
config.setCustomApiUrl(true);
5352
config.setApiUrl(CUSTOM_API_URL);
5453

5554
GitHubServerConfig config2 = new GitHubServerConfig(CREDENTIALS_ID);

‎src/test/java/org/jenkinsci/plugins/github/test/GHMockRule.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ public WireMockRule service() {
6363
*/
6464
public GitHubServerConfig serverConfig() {
6565
GitHubServerConfig conf = new GitHubServerConfig("creds");
66-
conf.setCustomApiUrl(true);
6766
conf.setApiUrl("http://localhost:" + service().port());
6867
return conf;
6968
}

0 commit comments

Comments
 (0)
0