From 69043bea74cf61080837ffa6a16fafff4bc815bf Mon Sep 17 00:00:00 2001 From: Wouter Born Date: Fri, 10 Dec 2021 23:33:13 +0100 Subject: [PATCH] Use Accept-Language header for locale in ConfigurableServiceResource (#2602) Most RESTResources already use the LocaleService to get the locale based on the Accept-Language header and use the system locale only as fallback. Because the ConfigurableServiceResource does not do this, it results in mixed up languages whenever your browser locale is not the same as the configured system locale. See: * https://community.openhab.org/t/language-support-question-regional-settings-language-vs-browser-language/128010 * https://github.com/openhab/openhab-webui/issues/1083 Signed-off-by: Wouter Born --- .../service/ConfigurableServiceResource.java | 64 +++++++++++-------- .../ConfigurableServiceResourceOSGiTest.java | 12 ++-- 2 files changed, 44 insertions(+), 32 deletions(-) diff --git a/bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/internal/service/ConfigurableServiceResource.java b/bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/internal/service/ConfigurableServiceResource.java index 4a7f09674b6..e7ed5493323 100644 --- a/bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/internal/service/ConfigurableServiceResource.java +++ b/bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/internal/service/ConfigurableServiceResource.java @@ -20,6 +20,7 @@ import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.stream.Collectors; @@ -27,6 +28,7 @@ import javax.ws.rs.Consumes; import javax.ws.rs.DELETE; import javax.ws.rs.GET; +import javax.ws.rs.HeaderParam; import javax.ws.rs.PUT; import javax.ws.rs.Path; import javax.ws.rs.PathParam; @@ -46,8 +48,8 @@ import org.openhab.core.config.core.ConfigurableServiceUtil; import org.openhab.core.config.core.Configuration; import org.openhab.core.i18n.I18nUtil; -import org.openhab.core.i18n.LocaleProvider; import org.openhab.core.i18n.TranslationProvider; +import org.openhab.core.io.rest.LocaleService; import org.openhab.core.io.rest.RESTConstants; import org.openhab.core.io.rest.RESTResource; import org.openhab.core.io.rest.core.config.ConfigurationService; @@ -109,7 +111,7 @@ public class ConfigurableServiceResource implements RESTResource { private final ConfigDescriptionRegistry configDescRegistry; private final ConfigurationService configurationService; private final TranslationProvider i18nProvider; - private final LocaleProvider localeProvider; + private final LocaleService localeService; @Activate public ConfigurableServiceResource( // @@ -117,20 +119,22 @@ public ConfigurableServiceResource( // final @Reference ConfigurationService configurationService, // final @Reference ConfigDescriptionRegistry configDescRegistry, // final @Reference TranslationProvider translationProvider, // - final @Reference LocaleProvider localeProvider) { + final @Reference LocaleService localeService) { this.bundleContext = bundleContext; this.configDescRegistry = configDescRegistry; this.configurationService = configurationService; this.i18nProvider = translationProvider; - this.localeProvider = localeProvider; + this.localeService = localeService; } @GET @Produces({ MediaType.APPLICATION_JSON }) @Operation(operationId = "getServices", summary = "Get all configurable services.", responses = { @ApiResponse(responseCode = "200", description = "OK", content = @Content(array = @ArraySchema(schema = @Schema(implementation = ConfigurableServiceDTO.class)))) }) - public List getAll() { - return getConfigurableServices(); + public List getAll( + @HeaderParam("Accept-Language") @Parameter(description = "language") @Nullable String language) { + Locale locale = localeService.getLocale(language); + return getConfigurableServices(locale); } @GET @@ -139,8 +143,11 @@ public List getAll() { @Operation(operationId = "getServicesById", summary = "Get configurable service for given service ID.", responses = { @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = ConfigurableServiceDTO.class))), @ApiResponse(responseCode = "404", description = "Not found") }) - public Response getById(@PathParam("serviceId") @Parameter(description = "service ID") String serviceId) { - ConfigurableServiceDTO configurableService = getServiceById(serviceId); + public Response getById( + @HeaderParam("Accept-Language") @Parameter(description = "language") @Nullable String language, + @PathParam("serviceId") @Parameter(description = "service ID") String serviceId) { + Locale locale = localeService.getLocale(language); + ConfigurableServiceDTO configurableService = getServiceById(serviceId, locale); if (configurableService != null) { return Response.ok(configurableService).build(); } else { @@ -148,13 +155,13 @@ public Response getById(@PathParam("serviceId") @Parameter(description = "servic } } - private @Nullable ConfigurableServiceDTO getServiceById(String serviceId) { - ConfigurableServiceDTO multiService = getMultiConfigServiceById(serviceId); + private @Nullable ConfigurableServiceDTO getServiceById(String serviceId, Locale locale) { + ConfigurableServiceDTO multiService = getMultiConfigServiceById(serviceId, locale); if (multiService != null) { return multiService; } - List configurableServices = getConfigurableServices(); + List configurableServices = getConfigurableServices(locale); for (ConfigurableServiceDTO configurableService : configurableServices) { if (configurableService.id.equals(serviceId)) { return configurableService; @@ -163,10 +170,10 @@ public Response getById(@PathParam("serviceId") @Parameter(description = "servic return null; } - private @Nullable ConfigurableServiceDTO getMultiConfigServiceById(String serviceId) { + private @Nullable ConfigurableServiceDTO getMultiConfigServiceById(String serviceId, Locale locale) { String filter = "(&(" + Constants.SERVICE_PID + "=" + serviceId + ")(" + ConfigurationAdmin.SERVICE_FACTORYPID + "=*))"; - List services = getServicesByFilter(filter); + List services = getServicesByFilter(filter, locale); if (services.size() == 1) { return services.get(0); } @@ -179,13 +186,15 @@ public Response getById(@PathParam("serviceId") @Parameter(description = "servic @Operation(operationId = "getServiceContext", summary = "Get existing multiple context service configurations for the given factory PID.", responses = { @ApiResponse(responseCode = "200", description = "OK", content = @Content(array = @ArraySchema(schema = @Schema(implementation = ConfigurableServiceDTO.class)))) }) public List getMultiConfigServicesByFactoryPid( + @HeaderParam("Accept-Language") @Parameter(description = "language") @Nullable String language, @PathParam("serviceId") @Parameter(description = "service ID") String serviceId) { - return collectServicesById(serviceId); + Locale locale = localeService.getLocale(language); + return collectServicesById(serviceId, locale); } - private List collectServicesById(String serviceId) { + private List collectServicesById(String serviceId, Locale locale) { String filter = "(" + ConfigurationAdmin.SERVICE_FACTORYPID + "=" + serviceId + ")"; - return getServicesByFilter(filter); + return getServicesByFilter(filter, locale); } @GET @@ -213,11 +222,15 @@ public Response getConfiguration(@PathParam("serviceId") @Parameter(description @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = String.class))), @ApiResponse(responseCode = "204", description = "No old configuration"), @ApiResponse(responseCode = "500", description = "Configuration can not be updated due to internal error") }) - public Response updateConfiguration(@PathParam("serviceId") @Parameter(description = "service ID") String serviceId, + public Response updateConfiguration( + @HeaderParam("Accept-Language") @Parameter(description = "language") @Nullable String language, + @PathParam("serviceId") @Parameter(description = "service ID") String serviceId, @Nullable Map configuration) { + Locale locale = localeService.getLocale(language); try { Configuration oldConfiguration = configurationService.get(serviceId); - configurationService.update(serviceId, new Configuration(normalizeConfiguration(configuration, serviceId))); + configurationService.update(serviceId, + new Configuration(normalizeConfiguration(configuration, serviceId, locale))); return oldConfiguration != null ? Response.ok(oldConfiguration.getProperties()).build() : Response.noContent().build(); } catch (IOException ex) { @@ -227,12 +240,12 @@ public Response updateConfiguration(@PathParam("serviceId") @Parameter(descripti } private @Nullable Map normalizeConfiguration(@Nullable Map properties, - String serviceId) { + String serviceId, Locale locale) { if (properties == null || properties.isEmpty()) { return properties; } - ConfigurableServiceDTO service = getServiceById(serviceId); + ConfigurableServiceDTO service = getServiceById(serviceId, locale); if (service == null) { return properties; } @@ -272,7 +285,7 @@ public Response deleteConfiguration( } } - private List getServicesByFilter(String filter) { + private List getServicesByFilter(String filter, Locale locale) { List services = new ArrayList<>(); ServiceReference[] serviceReferences = null; try { @@ -296,8 +309,7 @@ private List getServicesByFilter(String filter) { String key = I18nUtil.stripConstantOr(defaultLabel, () -> inferKey(configurableService.description_uri(), "label")); - String label = i18nProvider.getText(serviceReference.getBundle(), key, defaultLabel, - localeProvider.getLocale()); + String label = i18nProvider.getText(serviceReference.getBundle(), key, defaultLabel, locale); String category = configurableService.category(); @@ -335,11 +347,11 @@ private List getServicesByFilter(String filter) { return configDescriptionURI; } - private List getConfigurableServices() { + private List getConfigurableServices(Locale locale) { List services = new ArrayList<>(); - services.addAll(getServicesByFilter(ConfigurableServiceUtil.CONFIGURABLE_SERVICE_FILTER)); - services.addAll(getServicesByFilter(ConfigurableServiceUtil.CONFIGURABLE_MULTI_CONFIG_SERVICE_FILTER)); + services.addAll(getServicesByFilter(ConfigurableServiceUtil.CONFIGURABLE_SERVICE_FILTER, locale)); + services.addAll(getServicesByFilter(ConfigurableServiceUtil.CONFIGURABLE_MULTI_CONFIG_SERVICE_FILTER, locale)); return services; } diff --git a/itests/org.openhab.core.io.rest.core.tests/src/main/java/org/openhab/core/io/rest/core/internal/service/ConfigurableServiceResourceOSGiTest.java b/itests/org.openhab.core.io.rest.core.tests/src/main/java/org/openhab/core/io/rest/core/internal/service/ConfigurableServiceResourceOSGiTest.java index 30159eb0949..126b17d8947 100644 --- a/itests/org.openhab.core.io.rest.core.tests/src/main/java/org/openhab/core/io/rest/core/internal/service/ConfigurableServiceResourceOSGiTest.java +++ b/itests/org.openhab.core.io.rest.core.tests/src/main/java/org/openhab/core/io/rest/core/internal/service/ConfigurableServiceResourceOSGiTest.java @@ -77,7 +77,7 @@ public void setUp() throws Exception { @Test public void assertGetConfigurableServicesWorks() { - int num = configurableServiceResource.getAll().size(); + int num = configurableServiceResource.getAll("en").size(); Hashtable properties = new Hashtable<>(); properties.put("service.pid", "pid"); @@ -87,7 +87,7 @@ public void assertGetConfigurableServicesWorks() { registerService(mock(SomeServiceInterface.class), properties); - List configurableServices = configurableServiceResource.getAll(); + List configurableServices = configurableServiceResource.getAll("en"); assertThat(configurableServices.size(), is(num + 1)); Optional optionalService = configurableServices.stream() @@ -105,7 +105,7 @@ public void assertGetConfigurableServicesWorks() { @Test public void assertComponentNameFallbackWorks() { - int num = configurableServiceResource.getAll().size(); + int num = configurableServiceResource.getAll("en").size(); Hashtable properties = new Hashtable<>(); properties.put("component.name", "component.name"); @@ -113,7 +113,7 @@ public void assertComponentNameFallbackWorks() { registerService(mock(SomeServiceInterface.class), properties); - List configurableServices = configurableServiceResource.getAll(); + List configurableServices = configurableServiceResource.getAll("en"); assertThat(configurableServices.size(), is(num + 1)); Optional optionalService = configurableServices.stream() @@ -135,7 +135,7 @@ public void assertConfigurationManagementWorks() { Map newConfiguration = new HashMap<>(); newConfiguration.put("a", "b"); - response = configurableServiceResource.updateConfiguration("id", newConfiguration); + response = configurableServiceResource.updateConfiguration("en", "id", newConfiguration); assertThat(response.getStatus(), is(204)); response = configurableServiceResource.getConfiguration("id"); @@ -143,7 +143,7 @@ public void assertConfigurationManagementWorks() { assertThat(((Map) response.getEntity()).get("a"), is(equalTo("b"))); newConfiguration.put("a", "c"); - response = configurableServiceResource.updateConfiguration("id", newConfiguration); + response = configurableServiceResource.updateConfiguration("en", "id", newConfiguration); assertThat(response.getStatus(), is(200)); assertThat(((Map) response.getEntity()).get("a"), is(equalTo("b")));