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

Duplicate code for i18n in automation.providers and config.core bundles #922

Closed
tomhoefer opened this issue Jan 26, 2016 · 22 comments
Closed

Comments

@tomhoefer
Copy link
Contributor

Before the contribution of the automation bundles the config.core bundle was the single point in ESH to internationalize config descriptions and their parameters. Now the automation.providers bundle contains also an utility to internationalize config description parameters. This utility should be removed and the utility of the config.core bundle should be used instead.

One problem having two i18n utilities for the configuration description parameters is that is not possible to internationalize the unit and the unit label with the i18n utility of the automation.providers bundle. The reason for this is that the unit.properties is only available in the config.core bundle.

@kaikreuzer
Copy link
Contributor

@danchom @marinmitev Could you have a look at this? Do we have a way to remove the duplicate code?

@marinmitev
Copy link
Contributor

@adimova is working on this, it seems to be possible to avoid the duplication

@marinmitev
Copy link
Contributor

using the URI objects from org.eclipse.smarthome.config.core.i18n.ConfigDescriptionI18nUtil adds limitation to the characters used in the rules/moduletypes/templates UIDs. Such an example is the '_' symbol which is used in several places: ItemStateEvent_ON_Condition in file /org.eclipse.smarthome.automation.module.core/ESH-INF/automation/moduletypes/ItemEventConditionModuleTypeDefinition.json
This leads to runtime exceptions URISyntaxException and we will need to validate the UID objects.
According the RFC 2396 the allowed symbols for URI scheme are
scheme = alpha *( alpha | digit | "+" | "-" | "." )
and we should use the same limitation for the UIDs. @kaikreuzer do you agree ?

adimova pushed a commit to adimova/smarthome that referenced this issue Feb 5, 2016
…core bundles eclipse-archived#922

Signed-off-by: Ana Dimova <a.dimova@prosyst.bg>
@kaikreuzer
Copy link
Contributor

So far, we defined UIDs to be of this pattern: https://github.com/eclipse/smarthome/blob/d0dde874a4e8024b9b4c075720a986d68587bf06/bundles/core/org.eclipse.smarthome.core.thing/src/main/java/org/eclipse/smarthome/core/thing/UID.java#L24
This is imho valid within URIs and I would not like to have "+" or "." as characters within a UID.

Where exactly do you come across exceptions when using underscores in the UID?

@adimova
Copy link
Contributor

adimova commented Feb 8, 2016

I defined keys for localization of the automation objects:
module-type.{moduleTypeUID}.label;
module-type.{moduleTypeUID}.description;
module-type.{moduleTypeUID}.output.{InputName}.label;
module-type.{moduleTypeUID}.output.{InputName}.description;
module-type.{moduleTypeUID}.output.{OutputName}.label;
module-type.{moduleTypeUID}.output.{OutputName}.description;
module-type.{moduleTypeUID}.config.param.{parameterName}.label;
module-type.{moduleTypeUID}.config.param.{parameterName}.description;
...
rule-template.{RuleTemplateUID}.label;
rule-template.{RuleTemplateUID}.description;
rule-template.{RuleTemplateUID}.config.param.{parameterName}.label;
rule-template.{RuleTemplateUID}.config.param.{parameterName}.description;
...

I construct the configDescriptionURI as module-type.{moduleTypeUID}:param to receive the keys:

module-type.{moduleTypeUID}.config.param.{parameterName}.label;
module-type.{moduleTypeUID}.config.param.{parameterName}.description;

and as rule-template.{RuleTemplateUID}:param to receive the keys:

rule-template.{RuleTemplateUID}.config.param.{parameterName}.label;
rule-template.{RuleTemplateUID}.config.param.{parameterName}.description;

rule-template.{RuleTemplateUID} and module-type.{moduleTypeUID} are treated as scheme in ConfigDescriptionI18nUtil.inferKey(URI configDescriptionURI, String parameterName, String lastSegment) and this leads to runtime exceptions URISyntaxException

@tomhoefer
Copy link
Contributor Author

Could you pls provide an example of {RuleTemplateUID} and {moduleTypeUID} which throws an URISyntaxException... I would like to reproduce it...

@adimova
Copy link
Contributor

adimova commented Feb 9, 2016

if you use the commit c0dc375 and run the configuration "SmartHome Runtime", use the console command "smarthome automation listModuleTypes", you will receive the exception message:
java.net.URISyntaxException: Illegal character in scheme name at index 26: module-type.ItemStateEvent_ON_Condition:param
at java.net.URI$Parser.fail(URI.java:2848)
at java.net.URI$Parser.checkChars(URI.java:3021)
at java.net.URI$Parser.parse(URI.java:3048)
at java.net.URI.(URI.java:588)
at org.eclipse.smarthome.automation.internal.core.provider.AbstractResourceBundleProvider.getLocalizedConfigurationDescription(AbstractResourceBundleProvider.java:425)
at org.eclipse.smarthome.automation.internal.core.provider.ModuleTypeResourceBundleProvider.getPerLocale(ModuleTypeResourceBundleProvider.java:292)
at org.eclipse.smarthome.automation.internal.core.provider.ModuleTypeResourceBundleProvider.getModuleTypes(ModuleTypeResourceBundleProvider.java:184)
at org.eclipse.smarthome.automation.core.internal.type.ModuleTypeManager.getAll(ModuleTypeManager.java:145)
at org.eclipse.smarthome.automation.core.internal.type.ModuleTypeRegistryImpl.getAll(ModuleTypeRegistryImpl.java:63)
at org.eclipse.smarthome.automation.internal.commands.AutomationCommandsPluggable.getModuleTypes(AutomationCommandsPluggable.java:275)
at org.eclipse.smarthome.automation.internal.commands.AutomationCommandList.listModuleTypes(AutomationCommandList.java:235)
at org.eclipse.smarthome.automation.internal.commands.AutomationCommandList.execute(AutomationCommandList.java:80)
at org.eclipse.smarthome.automation.internal.commands.AutomationCommands.executeCommand(AutomationCommands.java:471)
at org.eclipse.smarthome.automation.internal.commands.AutomationCommandsPluggable.execute(AutomationCommandsPluggable.java:185)
at org.eclipse.smarthome.io.console.ConsoleInterpreter.execute(ConsoleInterpreter.java:52)
...

which is in the file "ESH_INF/automation/moduletypes/ItemEventConditionModuleTypeDefinition.json" in bundle org.eclipse.smarthome.automation.module.core

@kaikreuzer
Copy link
Contributor

@tomhoefer I think the problem is if a module type UID contains an _ and we create a configDescriptionURI from that.
How do we do it for thing-types? How does the config description URI look like for them if they contain an underscore? The very same should imho then happen for the automation entities.

@tomhoefer
Copy link
Contributor Author

I have to check....

@tomhoefer
Copy link
Contributor Author

There is no special logic to build the config description URI from thing-types that consist of an underscore. So having an underscore in the thing-type will result in a config description URI with an underscore.

@kaikreuzer
Copy link
Contributor

@tomhoefer As @marinmitev points out in #922 (comment), underscores are not a valid character for URIs, so you are hence doing it wrong, which needs to be fixed. Is your code not using the constructor of URI? If it does, it should also fail with an exception.

@tomhoefer
Copy link
Contributor Author

tomhoefer commented Jun 13, 2016

According to chapter 2.3 of RFC 2396 the underscore belongs to the unreserved characters that are allowed in URIs

Furthermore there is also no problem to execute the following simple main operation:

public static void main(String[] args) throws URISyntaxException {
    String string = "hello:world_1";
    System.out.println(new URI(string));

    string = "http://www.math_1.uio.no/faq/compression-faq/part1.html";
    System.out.println(new URI(string));
}

Maybe I still don´t get the issue....

@tomhoefer
Copy link
Contributor Author

Ok... I see the problem now....

@adimova
Copy link
Contributor

adimova commented Jun 13, 2016

I made changes in the way I construct the keys for localization that fixes the problem for automation module

@tomhoefer
Copy link
Contributor Author

The problem is the underscore in the scheme.... So

he_llo:world

cannot be parsed. Is it possible to exchange / replace the underscore in the module type ID?

@adimova
Copy link
Contributor

adimova commented Jun 13, 2016

This was my first proposition but Kai regect it, so I changed the way to construct the keys. They are now:
module-type.{moduleTypeUID}.label;
module-type.{moduleTypeUID}.description;
module-type.input.{moduleTypeUID}.name.{InputName}.label;
module-type.input.{moduleTypeUID}.name.{InputName}.description;
module-type.output.{moduleTypeUID}.name.{OutputName}.label;
module-type.output.{moduleTypeUID}.name.{OutputName}.description;
module-type.config.{moduleTypeUID}.name.{parameterName}.label;
module-type.config.{moduleTypeUID}.name.{parameterName}.description;
...
rule-template.{RuleTemplateUID}.label;
rule-template.{RuleTemplateUID}.description;
rule-template.config.{RuleTemplateUID}.name.{parameterName}.label;
rule-template.config.{RuleTemplateUID}.name.{parameterName}.description;
...
Now the configDescriptionURI for module type looks like "module-type:{moduleTypeUID}.name" and for template - "rule-template:{RuleTemplateUID}.name"

@tomhoefer
Copy link
Contributor Author

your change fixes the URISyntaxException, correct?

@kaikreuzer
Copy link
Contributor

This was my first proposition but Kai regect it, so I changed the way to construct the keys.

I am very confused now. @adimova, where did you ever have an underscore within the URI scheme? Can you give a concrete example?

@adimova
Copy link
Contributor

adimova commented Jun 13, 2016

yes, now Module Type UIDs participate only to the Scheme Specific Part and can contain "_"

@adimova
Copy link
Contributor

adimova commented Jun 13, 2016

in the first way the constructed URI was module-type.ItemStateEvent_ON_Condition:param and the scheme was "module-type.ItemStateEvent_ON_Condition" and Scheme Specific Part - "param", now the URI will be "module-type:ItemStateEvent_ON_Condition.name" where the scheme will be "module-type" and the Scheme Specific Part - "ItemStateEvent_ON_Condition.name"

@kaikreuzer
Copy link
Contributor

Ok, so the "first way" never made sense, as the scheme should only be the entity type as it is given in https://www.eclipse.org/smarthome/documentation/development/bindings/xml-reference.html#xml-structure-for-configuration-descriptions.
So there is no issue after all about underscores. Sorry @tomhoefer to have you bothered with that!

adimova pushed a commit to adimova/smarthome that referenced this issue Jun 13, 2016
Signed-off-by: Ana Dimova <a.dimova@prosyst.bg>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants