Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

Added options for language & regional settings #3755

Merged
merged 1 commit into from Jun 29, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -3,6 +3,7 @@
<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-1.8"/>
<classpathentry kind="con" path="org.eclipse.pde.core.requiredPlugins"/>
<classpathentry kind="src" path="src/test/groovy"/>
<classpathentry kind="src" path="src/test/java"/>
<classpathentry exported="true" kind="con" path="GROOVY_DSL_SUPPORT"/>
<classpathentry kind="output" path="target/test-classes"/>
</classpath>
@@ -0,0 +1,59 @@
/**
* Copyright (c) 2014-2017 by the respective copyright holders.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-v10.html
*/
package org.eclipse.smarthome.config.core.internal.i18n;

import static org.junit.Assert.*;

import java.net.URI;
import java.util.Locale;

import org.eclipse.smarthome.config.core.ParameterOption;
import org.junit.Before;
import org.junit.Test;

/**
* Tests for {@link I18nConfigOptionsProvider}
*
* @author Simon Kaufmann - initial contribution and API
*
*/
public class I18nConfigOptionsProviderTest {

private I18nConfigOptionsProvider provider;
private ParameterOption expectedLangEN = new ParameterOption("en", "English");
private ParameterOption expectedLangFR = new ParameterOption("en", "anglais");
private ParameterOption expectedCntryEN = new ParameterOption("US", "United States");
private ParameterOption expectedCntryFR = new ParameterOption("US", "Etats-Unis");
private URI uriI18N;

@Before
public void setup() throws Exception {
provider = new I18nConfigOptionsProvider();
uriI18N = new URI("system:i18n");
}

@Test
public void testLanguage() throws Exception {
assertTrue(provider.getParameterOptions(uriI18N, "language", Locale.US).contains(expectedLangEN));
assertTrue(provider.getParameterOptions(uriI18N, "language", Locale.FRENCH).contains(expectedLangFR));
assertFalse(provider.getParameterOptions(uriI18N, "language", null).isEmpty());
}

@Test
public void testRegion() throws Exception {
assertTrue(provider.getParameterOptions(uriI18N, "region", Locale.US).contains(expectedCntryEN));
assertTrue(provider.getParameterOptions(uriI18N, "region", Locale.FRENCH).contains(expectedCntryFR));
assertFalse(provider.getParameterOptions(uriI18N, "region", null).isEmpty());
}

@Test
public void testUnknownParameter() throws Exception {
assertNull(provider.getParameterOptions(uriI18N, "unknown", Locale.US));
}

}
Expand Up @@ -15,6 +15,7 @@ Import-Package: com.google.common.base,
org.eclipse.smarthome.core.i18n,
org.osgi.framework,
org.osgi.service.component,
org.osgi.service.component.annotations;resolution:=optional,
org.slf4j
Export-Package: org.eclipse.smarthome.config.core,
org.eclipse.smarthome.config.core.dto,
Expand Down
@@ -0,0 +1 @@
/org.eclipse.smarthome.config.core.internal.i18n.I18nConfigOptionsProvider.xml
Expand Up @@ -45,4 +45,42 @@ public String toString() {
return this.getClass().getSimpleName() + " [value=\"" + value + "\", label=\"" + label + "\"]";
}

@Override
public int hashCode() {
final int prime = 31;
int result = 1;
result = prime * result + ((label == null) ? 0 : label.hashCode());
result = prime * result + ((value == null) ? 0 : value.hashCode());
return result;
}

@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (obj == null) {
return false;
}
if (getClass() != obj.getClass()) {
return false;
}
ParameterOption other = (ParameterOption) obj;
if (label == null) {
if (other.label != null) {
return false;
}
} else if (!label.equals(other.label)) {
return false;
}
if (value == null) {
if (other.value != null) {
return false;
}
} else if (!value.equals(other.value)) {
return false;
}
return true;
}

}
@@ -0,0 +1,51 @@
/**
* Copyright (c) 2014-2017 by the respective copyright holders.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-v10.html
*/
package org.eclipse.smarthome.config.core.internal.i18n;

import java.net.URI;
import java.util.Arrays;
import java.util.Collection;
import java.util.Comparator;
import java.util.Locale;
import java.util.function.Function;
import java.util.stream.Collectors;

import org.eclipse.smarthome.config.core.ConfigOptionProvider;
import org.eclipse.smarthome.config.core.ParameterOption;
import org.osgi.service.component.annotations.Component;

/**
* {@link ConfigOptionProvider} that provides a list of
*
* @author Simon Kaufmann - initial contribution and API
*
*/
@Component(immediate = true)
public class I18nConfigOptionsProvider implements ConfigOptionProvider {

@Override
public Collection<ParameterOption> getParameterOptions(URI uri, String param, Locale locale) {
if (uri.toString().equals("system:i18n")) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the contract doesn't specify any throws clause, we cannot add any invariants here. Hence we could do this -> "system:i18n".equals(String.valueOf(uri))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uri is a non-null argument (see caller)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a quick look on the contract and the contract doesn't specify uri to be non null. If you think, the callers will never use null, you can ignore this.

Locale translation = locale != null ? locale : Locale.getDefault();
if (param.equals("language")) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

param can also be null, we can do this -> "language".equals(param).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a param with a null name? how should that work?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No clue if param is never null in the callers. Just the contract was not clear about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, the contract in fact is pretty clear:
ConfigDescriptionParameter.java#L204, config-description-1.0.0.xsd#L59
It's just at the wrong place/not replicated everywhere (yet), depending on your perspective.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. I only looked into ConfigOptionProvider interface but its callers handle the invariants separately. I believe it would also be beneficial if such invariants can also be added to the interfaces.

return getAvailable(locale, l -> new ParameterOption(l.getLanguage(), l.getDisplayLanguage(translation)));
} else if (param.equals("region")) {
return getAvailable(locale, l -> new ParameterOption(l.getCountry(), l.getDisplayCountry(translation)));
} else if (param.equals("variant")) {
return getAvailable(locale, l -> new ParameterOption(l.getVariant(), l.getDisplayVariant(translation)));
}
}
return null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the contract, we need to return empty list.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right - javadoc and caller implementation don't match

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}

private Collection<ParameterOption> getAvailable(Locale locale, Function<Locale, ParameterOption> mapFunction) {
return Arrays.stream(Locale.getAvailableLocales()).map(l -> mapFunction.apply(l)).distinct()
.sorted(Comparator.comparing(a -> a.getLabel())).collect(Collectors.toList());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a syntactic sugar: ParameterOption::getLabel

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that's personal flavor, isn't it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

definitely. It's upto you.

}

}