Skip to content

Commit

Permalink
Use Accept-Language header for locale in ConfigurableServiceResource (o…
Browse files Browse the repository at this point in the history 
…penhab#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
* openhab/openhab-webui#1083

Signed-off-by: Wouter Born <[email protected]>
GitOrigin-RevId: 69043be
  • Loading branch information
@wborn @splatch
wborn authored and splatch committed Jul 12, 2023
1 parent 0fd78d1 commit f0dd0b0
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 32 deletions.
64 changes: 38 additions & 26 deletions ...main/java/org/openhab/core/io/rest/core/internal/service/ConfigurableServiceResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@
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;

import javax.annotation.security.RolesAllowed;
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;
Expand All @@ -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;
Expand Down Expand Up @@ -109,28 +111,30 @@ 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( //
final BundleContext bundleContext, //
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<ConfigurableServiceDTO> getAll() {
return getConfigurableServices();
public List<ConfigurableServiceDTO> getAll(
@HeaderParam("Accept-Language") @Parameter(description = "language") @Nullable String language) {
Locale locale = localeService.getLocale(language);
return getConfigurableServices(locale);
}

@GET
Expand All @@ -139,22 +143,25 @@ public List<ConfigurableServiceDTO> 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 {
return Response.status(404).build();
}
}

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<ConfigurableServiceDTO> configurableServices = getConfigurableServices();
List<ConfigurableServiceDTO> configurableServices = getConfigurableServices(locale);
for (ConfigurableServiceDTO configurableService : configurableServices) {
if (configurableService.id.equals(serviceId)) {
return configurableService;
Expand All @@ -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<ConfigurableServiceDTO> services = getServicesByFilter(filter);
List<ConfigurableServiceDTO> services = getServicesByFilter(filter, locale);
if (services.size() == 1) {
return services.get(0);
}
Expand All @@ -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<ConfigurableServiceDTO> 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<ConfigurableServiceDTO> collectServicesById(String serviceId) {
private List<ConfigurableServiceDTO> collectServicesById(String serviceId, Locale locale) {
String filter = "(" + ConfigurationAdmin.SERVICE_FACTORYPID + "=" + serviceId + ")";
return getServicesByFilter(filter);
return getServicesByFilter(filter, locale);
}

@GET
Expand Down Expand Up @@ -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<String, Object> 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) {
Expand All @@ -227,12 +240,12 @@ public Response updateConfiguration(@PathParam("serviceId") @Parameter(descripti
}

private @Nullable Map<String, Object> normalizeConfiguration(@Nullable Map<String, Object> 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;
}
Expand Down Expand Up @@ -272,7 +285,7 @@ public Response deleteConfiguration(
}
}

private List<ConfigurableServiceDTO> getServicesByFilter(String filter) {
private List<ConfigurableServiceDTO> getServicesByFilter(String filter, Locale locale) {
List<ConfigurableServiceDTO> services = new ArrayList<>();
ServiceReference<?>[] serviceReferences = null;
try {
Expand All @@ -296,8 +309,7 @@ private List<ConfigurableServiceDTO> 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();

Expand Down Expand Up @@ -335,11 +347,11 @@ private List<ConfigurableServiceDTO> getServicesByFilter(String filter) {
return configDescriptionURI;
}

private List<ConfigurableServiceDTO> getConfigurableServices() {
private List<ConfigurableServiceDTO> getConfigurableServices(Locale locale) {
List<ConfigurableServiceDTO> 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;
}
Expand Down
12 changes: 6 additions & 6 deletions ...a/org/openhab/core/io/rest/core/internal/service/ConfigurableServiceResourceOSGiTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, Object> properties = new Hashtable<>();
properties.put("service.pid", "pid");
Expand All @@ -87,7 +87,7 @@ public void assertGetConfigurableServicesWorks() {

registerService(mock(SomeServiceInterface.class), properties);

List<ConfigurableServiceDTO> configurableServices = configurableServiceResource.getAll();
List<ConfigurableServiceDTO> configurableServices = configurableServiceResource.getAll("en");
assertThat(configurableServices.size(), is(num + 1));

Optional<ConfigurableServiceDTO> optionalService = configurableServices.stream()
Expand All @@ -105,15 +105,15 @@ public void assertGetConfigurableServicesWorks() {

@Test
public void assertComponentNameFallbackWorks() {
int num = configurableServiceResource.getAll().size();
int num = configurableServiceResource.getAll("en").size();

Hashtable<String, Object> properties = new Hashtable<>();
properties.put("component.name", "component.name");
properties.put("service.config.description.uri", "someuri");

registerService(mock(SomeServiceInterface.class), properties);

List<ConfigurableServiceDTO> configurableServices = configurableServiceResource.getAll();
List<ConfigurableServiceDTO> configurableServices = configurableServiceResource.getAll("en");
assertThat(configurableServices.size(), is(num + 1));

Optional<ConfigurableServiceDTO> optionalService = configurableServices.stream()
Expand All @@ -135,15 +135,15 @@ public void assertConfigurationManagementWorks() {

Map<String, Object> 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");
assertThat(response.getStatus(), is(200));
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")));

Expand Down

0 comments on commit f0dd0b0

Please sign in to comment.