From 2766eeeb20c3ad8f7c74ce41d7508c0a556ce9c2 Mon Sep 17 00:00:00 2001 From: scherler Date: Tue, 11 Oct 2016 08:48:44 +0100 Subject: [PATCH 01/11] Load i18n resource bundles from plugins if not found in jenkins core Signed-off-by: Thorsten Scherler --- .../java/jenkins/util/ResourceBundleUtil.java | 29 ++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/jenkins/util/ResourceBundleUtil.java b/core/src/main/java/jenkins/util/ResourceBundleUtil.java index a62c3fa7729d..06b8240cd247 100644 --- a/core/src/main/java/jenkins/util/ResourceBundleUtil.java +++ b/core/src/main/java/jenkins/util/ResourceBundleUtil.java @@ -23,6 +23,8 @@ */ package jenkins.util; +import hudson.PluginWrapper; +import jenkins.model.Jenkins; import net.sf.json.JSONObject; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; @@ -72,7 +74,23 @@ private ResourceBundleUtil() { return bundleJSON; } - ResourceBundle bundle = ResourceBundle.getBundle(baseName, locale); + ResourceBundle bundle = getBundle(baseName, locale, Jenkins.class.getClassLoader()); + if (bundle == null) { + // Not in Jenkins core. Check the plugins. + Jenkins jenkins = Jenkins.getInstance(); + if (jenkins != null) { + for (PluginWrapper plugin : jenkins.getPluginManager().getPlugins()) { + bundle = getBundle(baseName, locale, plugin.classLoader); + if (bundle != null) { + break; + } + } + } + } + if (bundle == null) { + throw new MissingResourceException("Can't find bundle for base name " + + baseName + ", locale " + locale, baseName + "_" + locale, ""); + } bundleJSON = toJSONObject(bundle); bundles.put(bundleKey, bundleJSON); @@ -80,6 +98,15 @@ private ResourceBundleUtil() { return bundleJSON; } + private static ResourceBundle getBundle(String baseName, Locale locale, ClassLoader classLoader) { + try { + return ResourceBundle.getBundle(baseName, locale, classLoader); + } catch (MissingResourceException e) { + // fall through and return null. + } + return null; + } + private static JSONObject toJSONObject(@Nonnull ResourceBundle bundle) { JSONObject json = new JSONObject(); for (String key : bundle.keySet()) { From 5641b6669e236cdf76a67fd3b89aeb7119335233 Mon Sep 17 00:00:00 2001 From: scherler Date: Tue, 11 Oct 2016 08:50:35 +0100 Subject: [PATCH 02/11] Issue 404 response for missing i18n resource bundles Currently issues a 200 with an "error" response payload. This change still issues the error response payload, but also sets the HTTP response. Signed-off-by: Thorsten Scherler --- .../main/java/hudson/util/HttpResponses.java | 26 +++++++++++++++++++ core/src/main/java/jenkins/I18n.java | 6 ++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/hudson/util/HttpResponses.java b/core/src/main/java/hudson/util/HttpResponses.java index 511a3f20e0cf..bde604fbc5f8 100644 --- a/core/src/main/java/hudson/util/HttpResponses.java +++ b/core/src/main/java/hudson/util/HttpResponses.java @@ -99,6 +99,18 @@ public static HttpResponse errorJSON(@Nonnull String message) { return new JSONObjectResponse().error(message); } + /** + * Set the response as an error response. + * @param message The error "message" set on the response. + * @param errorCode The HTTP error code to return; + * @return {@link this} object. + * + * @since TODO + */ + public static HttpResponse errorJSON(@Nonnull String message, int errorCode) { + return new JSONObjectResponse().error(message).setStatusCode(errorCode); + } + /** * {@link net.sf.json.JSONObject} response. * @@ -109,6 +121,7 @@ static class JSONObjectResponse implements HttpResponse { private static final Charset UTF8 = Charset.forName("UTF-8"); private final JSONObject jsonObject; + private Integer statusCode; /** * Create an empty "ok" response. @@ -145,6 +158,16 @@ static class JSONObjectResponse implements HttpResponse { this.jsonObject.put("data", JSONObject.fromObject(data)); } + /** + * Set the response status code. + * @param statusCode The status code. + * @return {@link this} object. + */ + public JSONObjectResponse setStatusCode(Integer statusCode) { + this.statusCode = statusCode; + return this; + } + /** * Set the response as an error response. * @param message The error "message" set on the response. @@ -175,6 +198,9 @@ static class JSONObjectResponse implements HttpResponse { @Override public void generateResponse(StaplerRequest req, StaplerResponse rsp, Object node) throws IOException, ServletException { byte[] bytes = jsonObject.toString().getBytes(UTF8); + if (statusCode != null) { + rsp.setStatus(statusCode); + } rsp.setContentType("application/json; charset=UTF-8"); rsp.setContentLength(bytes.length); rsp.getOutputStream().write(bytes); diff --git a/core/src/main/java/jenkins/I18n.java b/core/src/main/java/jenkins/I18n.java index d0f36ae3a4de..14220c1ec07f 100644 --- a/core/src/main/java/jenkins/I18n.java +++ b/core/src/main/java/jenkins/I18n.java @@ -32,7 +32,9 @@ import org.kohsuke.stapler.HttpResponse; import org.kohsuke.stapler.StaplerRequest; +import javax.servlet.http.HttpServletResponse; import java.util.Locale; +import java.util.MissingResourceException; /** * Internationalization REST (ish) API. @@ -106,8 +108,10 @@ public HttpResponse doResourceBundle(StaplerRequest request) { } return HttpResponses.okJSON(ResourceBundleUtil.getBundle(baseName, locale)); + } catch (MissingResourceException e) { + return HttpResponses.errorJSON(e.getMessage(), HttpServletResponse.SC_NOT_FOUND); } catch (Exception e) { - return HttpResponses.errorJSON(e.getMessage()); + return HttpResponses.errorJSON(e.getMessage(), HttpServletResponse.SC_INTERNAL_SERVER_ERROR); } } } From 3ee57ae1cfb5cd3ad792a7f12324f5938956613a Mon Sep 17 00:00:00 2001 From: Thorsten Scherler Date: Tue, 11 Oct 2016 15:33:06 +0200 Subject: [PATCH 03/11] [JENKINS-35845] Fix test since we return now a 404 --- test/src/test/java/jenkins/I18nTest.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/src/test/java/jenkins/I18nTest.java b/test/src/test/java/jenkins/I18nTest.java index a934422532e0..6983cc85c62e 100644 --- a/test/src/test/java/jenkins/I18nTest.java +++ b/test/src/test/java/jenkins/I18nTest.java @@ -49,9 +49,11 @@ public void test_baseName_unspecified() throws IOException, SAXException { @Test public void test_baseName_unknown() throws IOException, SAXException { - JSONObject response = jenkinsRule.getJSON("i18n/resourceBundle?baseName=com.acme.XyzWhatever").getJSONObject(); - Assert.assertEquals("error", response.getString("status")); - Assert.assertTrue(response.getString("message").contains("com.acme.XyzWhatever")); + try { + JSONObject response = jenkinsRule.getJSON("i18n/resourceBundle?baseName=com.acme.XyzWhatever").getJSONObject(); + } catch (Exception e) { + Assert.assertNotNull(e); + } } @Test From cf7d71b3e3b8e4daed7058d927420733327e7095 Mon Sep 17 00:00:00 2001 From: Thorsten Scherler Date: Thu, 13 Oct 2016 14:47:47 +0200 Subject: [PATCH 04/11] [JENKINS-35845] add test for getting locale from plugin. fix comments from oleg. --- .../main/java/hudson/util/HttpResponses.java | 64 ++++++++++++------- .../java/jenkins/util/ResourceBundleUtil.java | 51 ++++++++++----- test/src/test/java/jenkins/I18nTest.java | 12 +++- 3 files changed, 86 insertions(+), 41 deletions(-) diff --git a/core/src/main/java/hudson/util/HttpResponses.java b/core/src/main/java/hudson/util/HttpResponses.java index bde604fbc5f8..91c9596a3456 100644 --- a/core/src/main/java/hudson/util/HttpResponses.java +++ b/core/src/main/java/hudson/util/HttpResponses.java @@ -23,28 +23,30 @@ */ package hudson.util; +import java.io.File; +import java.io.IOException; +import java.nio.charset.Charset; +import java.util.Map; +import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; +import javax.servlet.ServletException; import net.sf.json.JSONArray; import net.sf.json.JSONObject; import org.kohsuke.stapler.HttpResponse; import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerResponse; -import javax.annotation.Nonnull; -import javax.servlet.ServletException; -import java.io.File; -import java.io.IOException; -import java.nio.charset.Charset; -import java.util.Map; - /** * Various {@link HttpResponse} implementations. * *

- * This class extends from Stapler so that we can move implementations from here to Stapler periodically. + * This class extends from Stapler so that we can move implementations from here + * to Stapler periodically. * * @author Kohsuke Kawaguchi */ public class HttpResponses extends org.kohsuke.stapler.HttpResponses { + public static HttpResponse staticResource(File f) throws IOException { return staticResource(f.toURI().toURL()); } @@ -60,6 +62,7 @@ public static HttpResponse okJSON() { /** * Create a response containing the supplied "data". + * * @param data The data. * * @since 2.0 @@ -70,6 +73,7 @@ public static HttpResponse okJSON(@Nonnull JSONObject data) { /** * Create a response containing the supplied "data". + * * @param data The data. * * @since 2.0 @@ -80,27 +84,30 @@ public static HttpResponse okJSON(@Nonnull JSONArray data) { /** * Create a response containing the supplied "data". + * * @param data The data. * * @since 2.0 */ - public static HttpResponse okJSON(@Nonnull Map data) { + public static HttpResponse okJSON(@Nonnull Map data) { return new JSONObjectResponse(data); } - /** - * Set the response as an error response. - * @param message The error "message" set on the response. - * @return {@link this} object. - * - * @since 2.0 - */ + /** + * Set the response as an error response. + * + * @param message The error "message" set on the response. + * @return {@link this} object. + * + * @since 2.0 + */ public static HttpResponse errorJSON(@Nonnull String message) { return new JSONObjectResponse().error(message); } /** * Set the response as an error response. + * * @param message The error "message" set on the response. * @param errorCode The HTTP error code to return; * @return {@link this} object. @@ -108,19 +115,21 @@ public static HttpResponse errorJSON(@Nonnull String message) { * @since TODO */ public static HttpResponse errorJSON(@Nonnull String message, int errorCode) { - return new JSONObjectResponse().error(message).setStatusCode(errorCode); + return new JSONObjectResponse().error(message).withStatusCode(errorCode); } /** * {@link net.sf.json.JSONObject} response. * - * @author tom.fennelly@gmail.com + * @author + * tom.fennelly@gmail.com */ static class JSONObjectResponse implements HttpResponse { private static final Charset UTF8 = Charset.forName("UTF-8"); private final JSONObject jsonObject; + @CheckForNull private Integer statusCode; /** @@ -133,6 +142,7 @@ static class JSONObjectResponse implements HttpResponse { /** * Create a response containing the supplied "data". + * * @param data The data. */ JSONObjectResponse(@Nonnull JSONObject data) { @@ -142,6 +152,7 @@ static class JSONObjectResponse implements HttpResponse { /** * Create a response containing the supplied "data". + * * @param data The data. */ JSONObjectResponse(@Nonnull JSONArray data) { @@ -151,29 +162,33 @@ static class JSONObjectResponse implements HttpResponse { /** * Create a response containing the supplied "data". + * * @param data The data. */ - JSONObjectResponse(@Nonnull Map data) { + JSONObjectResponse(@Nonnull Map data) { this(); this.jsonObject.put("data", JSONObject.fromObject(data)); } /** * Set the response status code. + * * @param statusCode The status code. * @return {@link this} object. */ - public JSONObjectResponse setStatusCode(Integer statusCode) { + public JSONObjectResponse withStatusCode(Integer statusCode) { this.statusCode = statusCode; return this; } /** * Set the response as an error response. + * * @param message The error "message" set on the response. * @return {@link this} object. */ - @Nonnull JSONObjectResponse error(@Nonnull String message) { + @Nonnull + JSONObjectResponse error(@Nonnull String message) { status("error"); this.jsonObject.put("message", message); return this; @@ -181,13 +196,16 @@ public JSONObjectResponse setStatusCode(Integer statusCode) { /** * Get the JSON response object. + * * @return The JSON response object. */ - @Nonnull JSONObject getJsonObject() { + @Nonnull + JSONObject getJsonObject() { return jsonObject; } - private @Nonnull JSONObjectResponse status(@Nonnull String status) { + private @Nonnull + JSONObjectResponse status(@Nonnull String status) { this.jsonObject.put("status", status); return this; } diff --git a/core/src/main/java/jenkins/util/ResourceBundleUtil.java b/core/src/main/java/jenkins/util/ResourceBundleUtil.java index 06b8240cd247..6d25aa000a91 100644 --- a/core/src/main/java/jenkins/util/ResourceBundleUtil.java +++ b/core/src/main/java/jenkins/util/ResourceBundleUtil.java @@ -24,20 +24,21 @@ package jenkins.util; import hudson.PluginWrapper; -import jenkins.model.Jenkins; -import net.sf.json.JSONObject; -import org.kohsuke.accmod.Restricted; -import org.kohsuke.accmod.restrictions.NoExternalUse; - -import javax.annotation.Nonnull; import java.util.Locale; import java.util.Map; import java.util.MissingResourceException; import java.util.ResourceBundle; import java.util.concurrent.ConcurrentHashMap; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import jenkins.model.Jenkins; +import net.sf.json.JSONObject; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; /** * Simple {@link java.util.ResourceBundle} utility class. + * * @author tom.fennelly@gmail.com * @since 2.0 */ @@ -51,22 +52,26 @@ private ResourceBundleUtil() { /** * Get a bundle JSON using the default Locale. + * * @param baseName The bundle base name. * @return The bundle JSON. * @throws MissingResourceException Missing resource bundle. */ - public static @Nonnull JSONObject getBundle(@Nonnull String baseName) throws MissingResourceException { + public static @Nonnull + JSONObject getBundle(@Nonnull String baseName) throws MissingResourceException { return getBundle(baseName, Locale.getDefault()); } /** * Get a bundle JSON using the supplied Locale. + * * @param baseName The bundle base name. * @param locale The Locale. * @return The bundle JSON. * @throws MissingResourceException Missing resource bundle. */ - public static @Nonnull JSONObject getBundle(@Nonnull String baseName, @Nonnull Locale locale) throws MissingResourceException { + public static @Nonnull + JSONObject getBundle(@Nonnull String baseName, @Nonnull Locale locale) throws MissingResourceException { String bundleKey = baseName + ":" + locale.toString(); JSONObject bundleJSON = bundles.get(bundleKey); @@ -77,13 +82,11 @@ private ResourceBundleUtil() { ResourceBundle bundle = getBundle(baseName, locale, Jenkins.class.getClassLoader()); if (bundle == null) { // Not in Jenkins core. Check the plugins. - Jenkins jenkins = Jenkins.getInstance(); - if (jenkins != null) { - for (PluginWrapper plugin : jenkins.getPluginManager().getPlugins()) { - bundle = getBundle(baseName, locale, plugin.classLoader); - if (bundle != null) { - break; - } + Jenkins jenkins = Jenkins.getInstance(); // will never return null + for (PluginWrapper plugin : jenkins.getPluginManager().getPlugins()) { + bundle = getBundle(baseName, locale, plugin.classLoader); + if (bundle != null) { + break; } } } @@ -98,7 +101,17 @@ private ResourceBundleUtil() { return bundleJSON; } - private static ResourceBundle getBundle(String baseName, Locale locale, ClassLoader classLoader) { + /** + * Get a plugin bundle using the supplied Locale and classLoader + * + * @param baseName The bundle base name. + * @param locale The Locale. + * @param classLoader The classLoader + * @return The bundle JSON. + * @throws MissingResourceException Missing resource bundle. + */ + private static @Nullable + ResourceBundle getBundle(@Nonnull String baseName, @Nonnull Locale locale, @Nonnull ClassLoader classLoader) { try { return ResourceBundle.getBundle(baseName, locale, classLoader); } catch (MissingResourceException e) { @@ -107,6 +120,12 @@ private static ResourceBundle getBundle(String baseName, Locale locale, ClassLoa return null; } + /** + * Create a JSON representation of a resource bundle + * + * @param bundle The resource bundle. + * @return The bundle JSON. + */ private static JSONObject toJSONObject(@Nonnull ResourceBundle bundle) { JSONObject json = new JSONObject(); for (String key : bundle.keySet()) { diff --git a/test/src/test/java/jenkins/I18nTest.java b/test/src/test/java/jenkins/I18nTest.java index 6983cc85c62e..321395f4c1bd 100644 --- a/test/src/test/java/jenkins/I18nTest.java +++ b/test/src/test/java/jenkins/I18nTest.java @@ -23,6 +23,7 @@ */ package jenkins; +import java.io.IOException; import net.sf.json.JSONObject; import org.junit.Assert; import org.junit.Rule; @@ -30,8 +31,6 @@ import org.jvnet.hudson.test.JenkinsRule; import org.xml.sax.SAXException; -import java.io.IOException; - /** * @author tom.fennelly@gmail.com */ @@ -56,6 +55,15 @@ public void test_baseName_unknown() throws IOException, SAXException { } } + @Test + public void test_baseName_plugin() throws IOException, SAXException { + // ssh-slaves plugin is installed by defect + JSONObject response = jenkinsRule.getJSON("i18n/resourceBundle?baseName=hudson.plugins.sshslaves.Messages").getJSONObject(); + Assert.assertEquals("ok", response.getString("status")); + JSONObject data = response.getJSONObject("data"); + Assert.assertEquals("The launch timeout must be a number.", data.getString("SSHConnector.LaunchTimeoutMustBeANumber")); + } + @Test public void test_valid() throws IOException, SAXException { JSONObject response = jenkinsRule.getJSON("i18n/resourceBundle?baseName=hudson.logging.Messages&language=de").getJSONObject(); From c05ac32b4470b168f7d3ba0628f9d690140f08dd Mon Sep 17 00:00:00 2001 From: Thorsten Scherler Date: Thu, 13 Oct 2016 16:02:14 +0200 Subject: [PATCH 05/11] [JENKINS-35845] Fix description --- test/src/test/java/jenkins/I18nTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/src/test/java/jenkins/I18nTest.java b/test/src/test/java/jenkins/I18nTest.java index 321395f4c1bd..fd9cafd9d7c3 100644 --- a/test/src/test/java/jenkins/I18nTest.java +++ b/test/src/test/java/jenkins/I18nTest.java @@ -57,7 +57,7 @@ public void test_baseName_unknown() throws IOException, SAXException { @Test public void test_baseName_plugin() throws IOException, SAXException { - // ssh-slaves plugin is installed by defect + // ssh-slaves plugin is installed by default JSONObject response = jenkinsRule.getJSON("i18n/resourceBundle?baseName=hudson.plugins.sshslaves.Messages").getJSONObject(); Assert.assertEquals("ok", response.getString("status")); JSONObject data = response.getJSONObject("data"); From 1fe461cb1c7b9cc354b5f79f58ab2e8bcc6053f2 Mon Sep 17 00:00:00 2001 From: Thorsten Scherler Date: Fri, 14 Oct 2016 21:15:44 +0200 Subject: [PATCH 06/11] [JENKINS-35845] Update PR with comments from Oleg --- .../main/java/hudson/util/HttpResponses.java | 33 ++++++------------- .../java/jenkins/util/ResourceBundleUtil.java | 25 +++++++------- 2 files changed, 21 insertions(+), 37 deletions(-) diff --git a/core/src/main/java/hudson/util/HttpResponses.java b/core/src/main/java/hudson/util/HttpResponses.java index 91c9596a3456..15369c0579d2 100644 --- a/core/src/main/java/hudson/util/HttpResponses.java +++ b/core/src/main/java/hudson/util/HttpResponses.java @@ -23,19 +23,19 @@ */ package hudson.util; -import java.io.File; -import java.io.IOException; -import java.nio.charset.Charset; -import java.util.Map; -import javax.annotation.CheckForNull; -import javax.annotation.Nonnull; -import javax.servlet.ServletException; import net.sf.json.JSONArray; import net.sf.json.JSONObject; import org.kohsuke.stapler.HttpResponse; import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerResponse; +import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; +import javax.servlet.ServletException; +import java.io.File; +import java.io.IOException; +import java.nio.charset.Charset; +import java.util.Map; /** * Various {@link HttpResponse} implementations. * @@ -46,14 +46,12 @@ * @author Kohsuke Kawaguchi */ public class HttpResponses extends org.kohsuke.stapler.HttpResponses { - public static HttpResponse staticResource(File f) throws IOException { return staticResource(f.toURI().toURL()); } /** * Create an empty "ok" response. - * * @since 2.0 */ public static HttpResponse okJSON() { @@ -62,7 +60,6 @@ public static HttpResponse okJSON() { /** * Create a response containing the supplied "data". - * * @param data The data. * * @since 2.0 @@ -73,7 +70,6 @@ public static HttpResponse okJSON(@Nonnull JSONObject data) { /** * Create a response containing the supplied "data". - * * @param data The data. * * @since 2.0 @@ -84,7 +80,6 @@ public static HttpResponse okJSON(@Nonnull JSONArray data) { /** * Create a response containing the supplied "data". - * * @param data The data. * * @since 2.0 @@ -107,7 +102,6 @@ public static HttpResponse errorJSON(@Nonnull String message) { /** * Set the response as an error response. - * * @param message The error "message" set on the response. * @param errorCode The HTTP error code to return; * @return {@link this} object. @@ -121,8 +115,7 @@ public static HttpResponse errorJSON(@Nonnull String message, int errorCode) { /** * {@link net.sf.json.JSONObject} response. * - * @author - * tom.fennelly@gmail.com + * @author tom.fennelly@gmail.com */ static class JSONObjectResponse implements HttpResponse { @@ -142,7 +135,6 @@ static class JSONObjectResponse implements HttpResponse { /** * Create a response containing the supplied "data". - * * @param data The data. */ JSONObjectResponse(@Nonnull JSONObject data) { @@ -152,7 +144,6 @@ static class JSONObjectResponse implements HttpResponse { /** * Create a response containing the supplied "data". - * * @param data The data. */ JSONObjectResponse(@Nonnull JSONArray data) { @@ -162,7 +153,6 @@ static class JSONObjectResponse implements HttpResponse { /** * Create a response containing the supplied "data". - * * @param data The data. */ JSONObjectResponse(@Nonnull Map data) { @@ -172,23 +162,20 @@ static class JSONObjectResponse implements HttpResponse { /** * Set the response status code. - * * @param statusCode The status code. * @return {@link this} object. */ - public JSONObjectResponse withStatusCode(Integer statusCode) { + public JSONObjectResponse withStatusCode(@CheckForNull Integer statusCode) { this.statusCode = statusCode; return this; } /** * Set the response as an error response. - * * @param message The error "message" set on the response. * @return {@link this} object. */ - @Nonnull - JSONObjectResponse error(@Nonnull String message) { + @Nonnull JSONObjectResponse error(@Nonnull String message) { status("error"); this.jsonObject.put("message", message); return this; diff --git a/core/src/main/java/jenkins/util/ResourceBundleUtil.java b/core/src/main/java/jenkins/util/ResourceBundleUtil.java index 6d25aa000a91..3adafbfb3b20 100644 --- a/core/src/main/java/jenkins/util/ResourceBundleUtil.java +++ b/core/src/main/java/jenkins/util/ResourceBundleUtil.java @@ -23,22 +23,23 @@ */ package jenkins.util; +import net.sf.json.JSONObject; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; + import hudson.PluginWrapper; +import java.util.logging.Logger; import java.util.Locale; import java.util.Map; import java.util.MissingResourceException; import java.util.ResourceBundle; import java.util.concurrent.ConcurrentHashMap; +import javax.annotation.CheckForNull; import javax.annotation.Nonnull; -import javax.annotation.Nullable; import jenkins.model.Jenkins; -import net.sf.json.JSONObject; -import org.kohsuke.accmod.Restricted; -import org.kohsuke.accmod.restrictions.NoExternalUse; /** * Simple {@link java.util.ResourceBundle} utility class. - * * @author tom.fennelly@gmail.com * @since 2.0 */ @@ -52,26 +53,22 @@ private ResourceBundleUtil() { /** * Get a bundle JSON using the default Locale. - * * @param baseName The bundle base name. * @return The bundle JSON. * @throws MissingResourceException Missing resource bundle. */ - public static @Nonnull - JSONObject getBundle(@Nonnull String baseName) throws MissingResourceException { + public static @Nonnull JSONObject getBundle(@Nonnull String baseName) throws MissingResourceException { return getBundle(baseName, Locale.getDefault()); } /** * Get a bundle JSON using the supplied Locale. - * * @param baseName The bundle base name. * @param locale The Locale. * @return The bundle JSON. * @throws MissingResourceException Missing resource bundle. */ - public static @Nonnull - JSONObject getBundle(@Nonnull String baseName, @Nonnull Locale locale) throws MissingResourceException { + public static @Nonnull JSONObject getBundle(@Nonnull String baseName, @Nonnull Locale locale) throws MissingResourceException { String bundleKey = baseName + ":" + locale.toString(); JSONObject bundleJSON = bundles.get(bundleKey); @@ -108,14 +105,14 @@ JSONObject getBundle(@Nonnull String baseName, @Nonnull Locale locale) throws Mi * @param locale The Locale. * @param classLoader The classLoader * @return The bundle JSON. - * @throws MissingResourceException Missing resource bundle. */ - private static @Nullable - ResourceBundle getBundle(@Nonnull String baseName, @Nonnull Locale locale, @Nonnull ClassLoader classLoader) { + private static @CheckForNull ResourceBundle getBundle(@Nonnull String baseName, @Nonnull Locale locale, @Nonnull ClassLoader classLoader) { try { return ResourceBundle.getBundle(baseName, locale, classLoader); } catch (MissingResourceException e) { // fall through and return null. + Logger logger = Logger.getLogger("jenkins.util.ResourceBundle"); + logger.info(e.getMessage()); } return null; } From 40f188599ad1df3fdd124c9aeab7d0e7a3a0ff92 Mon Sep 17 00:00:00 2001 From: Thorsten Scherler Date: Fri, 14 Oct 2016 21:43:04 +0200 Subject: [PATCH 07/11] [JENKINS-35845] Add feedback from tom --- core/src/main/java/hudson/util/HttpResponses.java | 7 ++----- core/src/main/java/jenkins/util/ResourceBundleUtil.java | 2 +- test/src/test/java/jenkins/I18nTest.java | 4 +++- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/hudson/util/HttpResponses.java b/core/src/main/java/hudson/util/HttpResponses.java index 15369c0579d2..cfdd1e1a866a 100644 --- a/core/src/main/java/hudson/util/HttpResponses.java +++ b/core/src/main/java/hudson/util/HttpResponses.java @@ -183,16 +183,13 @@ public JSONObjectResponse withStatusCode(@CheckForNull Integer statusCode) { /** * Get the JSON response object. - * * @return The JSON response object. */ - @Nonnull - JSONObject getJsonObject() { + @Nonnull JSONObject getJsonObject() { return jsonObject; } - private @Nonnull - JSONObjectResponse status(@Nonnull String status) { + private @Nonnull JSONObjectResponse status(@Nonnull String status) { this.jsonObject.put("status", status); return this; } diff --git a/core/src/main/java/jenkins/util/ResourceBundleUtil.java b/core/src/main/java/jenkins/util/ResourceBundleUtil.java index 3adafbfb3b20..0b10b3d7b221 100644 --- a/core/src/main/java/jenkins/util/ResourceBundleUtil.java +++ b/core/src/main/java/jenkins/util/ResourceBundleUtil.java @@ -46,6 +46,7 @@ @Restricted(NoExternalUse.class) public class ResourceBundleUtil { + private static final Logger logger = Logger.getLogger("jenkins.util.ResourceBundle"); private static final Map bundles = new ConcurrentHashMap<>(); private ResourceBundleUtil() { @@ -111,7 +112,6 @@ private ResourceBundleUtil() { return ResourceBundle.getBundle(baseName, locale, classLoader); } catch (MissingResourceException e) { // fall through and return null. - Logger logger = Logger.getLogger("jenkins.util.ResourceBundle"); logger.info(e.getMessage()); } return null; diff --git a/test/src/test/java/jenkins/I18nTest.java b/test/src/test/java/jenkins/I18nTest.java index fd9cafd9d7c3..fefc7e164e16 100644 --- a/test/src/test/java/jenkins/I18nTest.java +++ b/test/src/test/java/jenkins/I18nTest.java @@ -23,6 +23,7 @@ */ package jenkins; +import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; import java.io.IOException; import net.sf.json.JSONObject; import org.junit.Assert; @@ -50,8 +51,9 @@ public void test_baseName_unspecified() throws IOException, SAXException { public void test_baseName_unknown() throws IOException, SAXException { try { JSONObject response = jenkinsRule.getJSON("i18n/resourceBundle?baseName=com.acme.XyzWhatever").getJSONObject(); - } catch (Exception e) { + } catch (FailingHttpStatusCodeException e) { Assert.assertNotNull(e); + Assert.assertTrue(e.getMessage().contains("com.acme.XyzWhatever")); } } From 72cbdbb487750dcc404d3204e3270355cbd1c100 Mon Sep 17 00:00:00 2001 From: Thorsten Scherler Date: Mon, 17 Oct 2016 12:08:57 +0200 Subject: [PATCH 08/11] eslint - formating changes and fix offences --- .../main/java/hudson/util/HttpResponses.java | 25 ++++++++++--------- .../java/jenkins/util/ResourceBundleUtil.java | 4 +-- test/src/test/java/jenkins/I18nTest.java | 3 ++- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/core/src/main/java/hudson/util/HttpResponses.java b/core/src/main/java/hudson/util/HttpResponses.java index cfdd1e1a866a..0527b48f30c9 100644 --- a/core/src/main/java/hudson/util/HttpResponses.java +++ b/core/src/main/java/hudson/util/HttpResponses.java @@ -36,12 +36,12 @@ import java.io.IOException; import java.nio.charset.Charset; import java.util.Map; + /** * Various {@link HttpResponse} implementations. * *

- * This class extends from Stapler so that we can move implementations from here - * to Stapler periodically. + * This class extends from Stapler so that we can move implementations from here to Stapler periodically. * * @author Kohsuke Kawaguchi */ @@ -52,6 +52,7 @@ public static HttpResponse staticResource(File f) throws IOException { /** * Create an empty "ok" response. + * * @since 2.0 */ public static HttpResponse okJSON() { @@ -84,18 +85,18 @@ public static HttpResponse okJSON(@Nonnull JSONArray data) { * * @since 2.0 */ - public static HttpResponse okJSON(@Nonnull Map data) { + public static HttpResponse okJSON(@Nonnull Map data) { return new JSONObjectResponse(data); } - /** - * Set the response as an error response. - * - * @param message The error "message" set on the response. - * @return {@link this} object. - * - * @since 2.0 - */ + /** + * Set the response as an error response. + * + * @param message The error "message" set on the response. + * @return {@link this} object. + * + * @since 2.0 + */ public static HttpResponse errorJSON(@Nonnull String message) { return new JSONObjectResponse().error(message); } @@ -155,7 +156,7 @@ static class JSONObjectResponse implements HttpResponse { * Create a response containing the supplied "data". * @param data The data. */ - JSONObjectResponse(@Nonnull Map data) { + JSONObjectResponse(@Nonnull Map data) { this(); this.jsonObject.put("data", JSONObject.fromObject(data)); } diff --git a/core/src/main/java/jenkins/util/ResourceBundleUtil.java b/core/src/main/java/jenkins/util/ResourceBundleUtil.java index 0b10b3d7b221..44d06e8c501d 100644 --- a/core/src/main/java/jenkins/util/ResourceBundleUtil.java +++ b/core/src/main/java/jenkins/util/ResourceBundleUtil.java @@ -27,6 +27,8 @@ import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; +import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; import hudson.PluginWrapper; import java.util.logging.Logger; import java.util.Locale; @@ -34,8 +36,6 @@ import java.util.MissingResourceException; import java.util.ResourceBundle; import java.util.concurrent.ConcurrentHashMap; -import javax.annotation.CheckForNull; -import javax.annotation.Nonnull; import jenkins.model.Jenkins; /** diff --git a/test/src/test/java/jenkins/I18nTest.java b/test/src/test/java/jenkins/I18nTest.java index fefc7e164e16..992042827784 100644 --- a/test/src/test/java/jenkins/I18nTest.java +++ b/test/src/test/java/jenkins/I18nTest.java @@ -24,7 +24,6 @@ package jenkins; import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; -import java.io.IOException; import net.sf.json.JSONObject; import org.junit.Assert; import org.junit.Rule; @@ -32,6 +31,8 @@ import org.jvnet.hudson.test.JenkinsRule; import org.xml.sax.SAXException; +import java.io.IOException; + /** * @author tom.fennelly@gmail.com */ From 0b8c8cd6b753b7f6479f550e422048ec2deaa16f Mon Sep 17 00:00:00 2001 From: Thorsten Scherler Date: Mon, 17 Oct 2016 12:11:03 +0200 Subject: [PATCH 09/11] eslint - formating changes and fix offences --- core/src/main/java/hudson/util/HttpResponses.java | 1 - 1 file changed, 1 deletion(-) diff --git a/core/src/main/java/hudson/util/HttpResponses.java b/core/src/main/java/hudson/util/HttpResponses.java index 0527b48f30c9..210a25bca203 100644 --- a/core/src/main/java/hudson/util/HttpResponses.java +++ b/core/src/main/java/hudson/util/HttpResponses.java @@ -91,7 +91,6 @@ public static HttpResponse okJSON(@Nonnull Map data) { /** * Set the response as an error response. - * * @param message The error "message" set on the response. * @return {@link this} object. * From 0aab660268fc223664c2497aa95606a94547b74f Mon Sep 17 00:00:00 2001 From: Thorsten Scherler Date: Tue, 18 Oct 2016 18:17:23 +0200 Subject: [PATCH 10/11] [JENKINS-35845] remove code concerning 404 response. Fix resourceBundle test by prevent NPE to happen --- .../main/java/hudson/util/HttpResponses.java | 27 ------------------- core/src/main/java/jenkins/I18n.java | 6 +---- .../java/jenkins/util/ResourceBundleUtil.java | 12 +++++---- 3 files changed, 8 insertions(+), 37 deletions(-) diff --git a/core/src/main/java/hudson/util/HttpResponses.java b/core/src/main/java/hudson/util/HttpResponses.java index 210a25bca203..ccef89059e1a 100644 --- a/core/src/main/java/hudson/util/HttpResponses.java +++ b/core/src/main/java/hudson/util/HttpResponses.java @@ -100,18 +100,6 @@ public static HttpResponse errorJSON(@Nonnull String message) { return new JSONObjectResponse().error(message); } - /** - * Set the response as an error response. - * @param message The error "message" set on the response. - * @param errorCode The HTTP error code to return; - * @return {@link this} object. - * - * @since TODO - */ - public static HttpResponse errorJSON(@Nonnull String message, int errorCode) { - return new JSONObjectResponse().error(message).withStatusCode(errorCode); - } - /** * {@link net.sf.json.JSONObject} response. * @@ -122,8 +110,6 @@ static class JSONObjectResponse implements HttpResponse { private static final Charset UTF8 = Charset.forName("UTF-8"); private final JSONObject jsonObject; - @CheckForNull - private Integer statusCode; /** * Create an empty "ok" response. @@ -160,16 +146,6 @@ static class JSONObjectResponse implements HttpResponse { this.jsonObject.put("data", JSONObject.fromObject(data)); } - /** - * Set the response status code. - * @param statusCode The status code. - * @return {@link this} object. - */ - public JSONObjectResponse withStatusCode(@CheckForNull Integer statusCode) { - this.statusCode = statusCode; - return this; - } - /** * Set the response as an error response. * @param message The error "message" set on the response. @@ -200,9 +176,6 @@ public JSONObjectResponse withStatusCode(@CheckForNull Integer statusCode) { @Override public void generateResponse(StaplerRequest req, StaplerResponse rsp, Object node) throws IOException, ServletException { byte[] bytes = jsonObject.toString().getBytes(UTF8); - if (statusCode != null) { - rsp.setStatus(statusCode); - } rsp.setContentType("application/json; charset=UTF-8"); rsp.setContentLength(bytes.length); rsp.getOutputStream().write(bytes); diff --git a/core/src/main/java/jenkins/I18n.java b/core/src/main/java/jenkins/I18n.java index 14220c1ec07f..d0f36ae3a4de 100644 --- a/core/src/main/java/jenkins/I18n.java +++ b/core/src/main/java/jenkins/I18n.java @@ -32,9 +32,7 @@ import org.kohsuke.stapler.HttpResponse; import org.kohsuke.stapler.StaplerRequest; -import javax.servlet.http.HttpServletResponse; import java.util.Locale; -import java.util.MissingResourceException; /** * Internationalization REST (ish) API. @@ -108,10 +106,8 @@ public HttpResponse doResourceBundle(StaplerRequest request) { } return HttpResponses.okJSON(ResourceBundleUtil.getBundle(baseName, locale)); - } catch (MissingResourceException e) { - return HttpResponses.errorJSON(e.getMessage(), HttpServletResponse.SC_NOT_FOUND); } catch (Exception e) { - return HttpResponses.errorJSON(e.getMessage(), HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + return HttpResponses.errorJSON(e.getMessage()); } } } diff --git a/core/src/main/java/jenkins/util/ResourceBundleUtil.java b/core/src/main/java/jenkins/util/ResourceBundleUtil.java index 44d06e8c501d..edf8de5fab82 100644 --- a/core/src/main/java/jenkins/util/ResourceBundleUtil.java +++ b/core/src/main/java/jenkins/util/ResourceBundleUtil.java @@ -81,10 +81,12 @@ private ResourceBundleUtil() { if (bundle == null) { // Not in Jenkins core. Check the plugins. Jenkins jenkins = Jenkins.getInstance(); // will never return null - for (PluginWrapper plugin : jenkins.getPluginManager().getPlugins()) { - bundle = getBundle(baseName, locale, plugin.classLoader); - if (bundle != null) { - break; + if (jenkins != null) { + for (PluginWrapper plugin : jenkins.getPluginManager().getPlugins()) { + bundle = getBundle(baseName, locale, plugin.classLoader); + if (bundle != null) { + break; + } } } } @@ -112,7 +114,7 @@ private ResourceBundleUtil() { return ResourceBundle.getBundle(baseName, locale, classLoader); } catch (MissingResourceException e) { // fall through and return null. - logger.info(e.getMessage()); + logger.warning(e.getMessage()); } return null; } From 46b483ed190fef1ac39f5e9624706faa20b8c1e4 Mon Sep 17 00:00:00 2001 From: Thorsten Scherler Date: Thu, 20 Oct 2016 19:01:49 +0200 Subject: [PATCH 11/11] [JENKINS-35845] Link to issue on which we introduced the test --- test/src/test/java/jenkins/I18nTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/src/test/java/jenkins/I18nTest.java b/test/src/test/java/jenkins/I18nTest.java index 992042827784..5f2da8e7938c 100644 --- a/test/src/test/java/jenkins/I18nTest.java +++ b/test/src/test/java/jenkins/I18nTest.java @@ -32,6 +32,7 @@ import org.xml.sax.SAXException; import java.io.IOException; +import org.jvnet.hudson.test.Issue; /** * @author tom.fennelly@gmail.com @@ -58,6 +59,7 @@ public void test_baseName_unknown() throws IOException, SAXException { } } + @Issue("JENKINS-35270") @Test public void test_baseName_plugin() throws IOException, SAXException { // ssh-slaves plugin is installed by default