Skip to content

Commit 0926c54

Browse files
NadaharNadahar
authored
Rule conversion preparation (#5502)
* Make YAML Rule and RuleTemplate providers handle isolated models * Fix bug from #4633 * TemplateState was originally (during #4718 development) called TemplateStatus, and when changed, not every reference was included, which is corrected here * Add a new constructor to RuleBuilder that allows to make a "copy" of a Rule using a new UID, and add Javadocs to the constructors * Remove remnants of pre #4718 logic in Rule Javadocs * Make StateAndCommandProvider sets immutable and all getters static * Add /src.moved/ to .gitignore because it appears there after every build, constantly causing diffs. * Use the new constants in ModuleTypeAliases and fix a bug in typeToAlias() * Minor refactoring of rule template resolving * Correct ConfigDescriptionParameter Javadoc and create toString() in ModuleImpl to ease debugging Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>
1 parent 7d99e35 commit 0926c54

16 files changed

Lines changed: 472 additions & 128 deletions

File tree

‎bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/Rule.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,10 @@ public interface Rule extends Identifiable<String> {
5959
* This method is used to obtain the {@link RuleTemplate} identifier of the template the {@link Rule} was created
6060
* from. It will be used by the {@link RuleRegistry} to resolve the {@link Rule}: to validate the {@link Rule}'s
6161
* configuration, as well as to create and configure the {@link Rule}'s modules. If a {@link Rule} has not been
62-
* created from a template, or has been successfully resolved by the {@link RuleRegistry}, this method will return
63-
* {@code null}.
62+
* created from a template, this method will return {@code null}.
6463
*
6564
* @return the identifier of the {@link Rule}'s {@link RuleTemplate}, or {@code null} if the {@link Rule} has not
66-
* been created from a template, or has been successfully resolved by the {@link RuleRegistry}.
65+
* been created from a template.
6766
*/
6867
@Nullable
6968
String getTemplateUID();

‎bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/ModuleImpl.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,4 +155,19 @@ public Configuration getConfiguration() {
155155
public void setConfiguration(Configuration configuration) {
156156
this.configuration = new Configuration(configuration);
157157
}
158+
159+
@Override
160+
public String toString() {
161+
StringBuilder sb = new StringBuilder(60);
162+
sb.append(getClass().getSimpleName()).append(" [").append("id=").append(id).append(", ").append("typeUID=")
163+
.append(typeUID).append(", ");
164+
if (label != null) {
165+
sb.append("label=").append(label).append(", ");
166+
}
167+
if (description != null) {
168+
sb.append("description=").append(description).append(", ");
169+
}
170+
sb.append("configuration=").append(configuration).append("]");
171+
return sb.toString();
172+
}
158173
}

‎bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/RuleImpl.java‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public class RuleImpl implements Rule {
4949
protected Configuration configuration;
5050
protected List<ConfigDescriptionParameter> configDescriptions;
5151
protected @Nullable String templateUID;
52-
protected TemplateState templateStatus;
52+
protected TemplateState templateState;
5353
protected String uid;
5454
protected @Nullable String name;
5555
protected Set<String> tags;
@@ -87,12 +87,13 @@ public RuleImpl(@Nullable String uid) {
8787
* @param templateUID the {@link RuleTemplate} identifier of the template that will be used by the
8888
* {@link RuleRegistry} to validate the {@link Rule}'s configuration, as well as to create and configure
8989
* the {@link Rule}'s modules, or null if the {@link Rule} should not be created from a template.
90+
* @param templateState the {@link TemplateState} of the {@link Rule}.
9091
* @param visibility the {@link Rule}'s visibility
9192
*/
9293
public RuleImpl(@Nullable String uid, final @Nullable String name, final @Nullable String description,
9394
final @Nullable Set<String> tags, @Nullable List<Trigger> triggers, @Nullable List<Condition> conditions,
9495
@Nullable List<Action> actions, @Nullable List<ConfigDescriptionParameter> configDescriptions,
95-
@Nullable Configuration configuration, @Nullable String templateUID, TemplateState templateStatus,
96+
@Nullable Configuration configuration, @Nullable String templateUID, TemplateState templateState,
9697
@Nullable Visibility visibility) {
9798
this.uid = uid == null ? UUID.randomUUID().toString() : uid;
9899
this.name = name;
@@ -105,7 +106,7 @@ public RuleImpl(@Nullable String uid, final @Nullable String name, final @Nullab
105106
this.configuration = configuration == null ? new Configuration()
106107
: new Configuration(configuration.getProperties());
107108
this.templateUID = templateUID;
108-
this.templateStatus = templateStatus;
109+
this.templateState = templateState;
109110
this.visibility = visibility == null ? Visibility.VISIBLE : visibility;
110111
}
111112

@@ -130,16 +131,16 @@ public void setTemplateUID(@Nullable String templateUID) {
130131

131132
@Override
132133
public TemplateState getTemplateState() {
133-
return templateStatus;
134+
return templateState;
134135
}
135136

136137
/**
137138
* This method is used to specify the current rule template state.
138139
*
139140
* @param templateState the {@link TemplateState} to set.
140141
*/
141-
public void setTemplateStatus(TemplateState templateState) {
142-
this.templateStatus = Objects.requireNonNull(templateState);
142+
public void setTemplateState(TemplateState templateState) {
143+
this.templateState = Objects.requireNonNull(templateState);
143144
}
144145

145146
@Override

‎bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/RuleRegistryImpl.java‎

Lines changed: 20 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -367,13 +367,14 @@ public void regenerateFromTemplate(String ruleUID) {
367367
}
368368

369369
/**
370-
* The method checks if the rule has to be resolved by template or not. If the rule does not contain tempateUID it
371-
* returns same rule, otherwise it tries to resolve the rule created from template. If the template is available
372-
* the method creates a new rule based on triggers, conditions and actions from template. If the template is not
373-
* available returns the same rule.
370+
* The method checks if the rule has to be resolved by template or not. If the rule does not contain a
371+
* {@code templateUID} or the {@code templateState} is {@code NO_TEMPLATE} or {@code INSTANTIATED}, it returns the
372+
* same rule, otherwise it tries to resolve the rule created from template. If the template is available the method
373+
* creates a new rule based on triggers, conditions and actions from template. If the template is not available, it
374+
* returns the same rule.
374375
*
375376
* @param rule a rule defined by template.
376-
* @return the resolved rule(containing modules defined by the template) or not resolved rule, if the template is
377+
* @return the resolved rule (containing modules defined by the template) or not resolved rule, if the template is
377378
* missing.
378379
*/
379380
private Rule resolveRuleByTemplate(Rule rule) {
@@ -458,21 +459,19 @@ public void updated(Provider<Rule> provider, Rule oldElement, Rule element) {
458459

459460
@Override
460461
protected void onAddElement(Rule element) throws IllegalArgumentException {
461-
String uid = element.getUID();
462462
try {
463463
resolveConfigurations(element);
464464
} catch (IllegalArgumentException e) {
465-
logger.debug("Added rule '{}' is invalid", uid, e);
465+
logger.debug("Added rule '{}' is invalid", element.getUID(), e);
466466
}
467467
}
468468

469469
@Override
470470
protected void onUpdateElement(Rule oldElement, Rule element) throws IllegalArgumentException {
471-
String uid = element.getUID();
472471
try {
473472
resolveConfigurations(element);
474473
} catch (IllegalArgumentException e) {
475-
logger.debug("The new version of updated rule '{}' is invalid", uid, e);
474+
logger.debug("The new version of updated rule '{}' is invalid", element.getUID(), e);
476475
}
477476
}
478477

@@ -481,8 +480,9 @@ protected void onUpdateElement(Rule oldElement, Rule element) throws IllegalArgu
481480
*
482481
* @param rule the {@link Rule}, whose configuration values and module configuration values should be resolved and
483482
* normalized.
483+
* @throws IllegalArgumentException If the configuration is incorrect.
484484
*/
485-
private void resolveConfigurations(Rule rule) {
485+
private void resolveConfigurations(Rule rule) throws IllegalArgumentException {
486486
List<ConfigDescriptionParameter> configDescriptions = rule.getConfigurationDescriptions();
487487
Configuration configuration = rule.getConfiguration();
488488
ConfigurationNormalizer.normalizeConfiguration(configuration,
@@ -492,23 +492,27 @@ private void resolveConfigurations(Rule rule) {
492492
if (templateState == TemplateState.INSTANTIATED || templateState == TemplateState.NO_TEMPLATE) {
493493
String uid = rule.getUID();
494494
try {
495-
validateConfiguration(configDescriptions, new HashMap<>(configurationProperties));
495+
validateConfiguration(configDescriptions, configurationProperties);
496496
resolveModuleConfigReferences(rule.getModules(), configurationProperties);
497497
ConfigurationNormalizer.normalizeModuleConfigurations(rule.getModules(), moduleTypeRegistry);
498498
} catch (IllegalArgumentException e) {
499-
throw new IllegalArgumentException(String.format("The rule '%s' has incorrect configurations", uid), e);
499+
throw new IllegalArgumentException(
500+
String.format("The rule '%s' has incorrect configuration: %s", uid, e.getMessage()), e);
500501
}
501502
}
502503
}
503504

504505
/**
505506
* This method serves to validate the {@link Rule}s configuration values.
506507
*
507-
* @param rule the {@link Rule}, whose configuration values should be validated.
508+
* @param configDescriptions the configuration parameter descriptions used to validate the configuration.
509+
* @param configuration the configuration whose properties to validate.
510+
* @throws IllegalArgumentException If a required configuration property is missing.
508511
*/
509512
private void validateConfiguration(List<ConfigDescriptionParameter> configDescriptions,
510-
Map<String, Object> configurations) {
511-
if (configurations == null || configurations.isEmpty()) {
513+
Map<String, Object> configuration) throws IllegalArgumentException {
514+
Map<String, Object> config = configuration == null ? new HashMap<>() : new HashMap<>(configuration);
515+
if (config.isEmpty()) {
512516
if (isOptionalConfig(configDescriptions)) {
513517
return;
514518
} else {
@@ -526,15 +530,7 @@ private void validateConfiguration(List<ConfigDescriptionParameter> configDescri
526530
} else {
527531
for (ConfigDescriptionParameter configParameter : configDescriptions) {
528532
String configParameterName = configParameter.getName();
529-
processValue(configurations.remove(configParameterName), configParameter);
530-
}
531-
if (!configurations.isEmpty()) {
532-
StringBuilder statusDescription = new StringBuilder();
533-
String msg = " '%s';";
534-
for (String name : configurations.keySet()) {
535-
statusDescription.append(String.format(msg, name));
536-
}
537-
throw new IllegalArgumentException("Extra configuration properties: " + statusDescription.toString());
533+
processValue(config.remove(configParameterName), configParameter);
538534
}
539535
}
540536
}

‎bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/util/RuleBuilder.java‎

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,19 +69,54 @@ protected RuleBuilder(Rule rule) {
6969
this.description = rule.getDescription();
7070
}
7171

72-
public static RuleBuilder create(String ruleId) {
73-
Rule rule = new RuleImpl(ruleId);
72+
/**
73+
* Build a new {@link Rule} using the specified rule UID.
74+
*
75+
* @param ruleUid the UID to use.
76+
* @return The new {@link RuleBuilder}.
77+
*/
78+
public static RuleBuilder create(String ruleUid) {
79+
Rule rule = new RuleImpl(ruleUid);
7480
return new RuleBuilder(rule);
7581
}
7682

83+
/**
84+
* Build a new {@link Rule} based on an existing rule.
85+
*
86+
* @param r the {@link Rule} to base the builder on.
87+
* @return The new {@link RuleBuilder}.
88+
*/
7789
public static RuleBuilder create(Rule r) {
78-
return create(r.getUID()).withActions(r.getActions()).withConditions(r.getConditions())
90+
return create(r.getUID(), r);
91+
}
92+
93+
/**
94+
* Build a new {@link Rule} with a new UID, based on an existing rule.
95+
*
96+
* @param ruleUid the UID to use.
97+
* @param r the {@link Rule} to base the builder on.
98+
* @return The new {@link RuleBuilder}.
99+
*/
100+
public static RuleBuilder create(String ruleUid, Rule r) {
101+
return create(ruleUid).withActions(r.getActions()).withConditions(r.getConditions())
79102
.withTriggers(r.getTriggers()).withConfiguration(r.getConfiguration())
80103
.withConfigurationDescriptions(r.getConfigurationDescriptions()).withDescription(r.getDescription())
81104
.withName(r.getName()).withTags(r.getTags()).withTemplateUID(r.getTemplateUID())
82105
.withTemplateState(r.getTemplateState());
83106
}
84107

108+
/**
109+
* Build a new {@link Rule} from the specified {@link RuleTemplate} and {@link Configuration}. The resulting rule
110+
* will be ready for template placeholder substitution, but the placeholders won't actually have been substituted.
111+
* This constructor is only suitable for preparing to substitute placeholders.
112+
*
113+
* @param template the {@link RuleTemplate} to use.
114+
* @param uid the UID of the resulting {@link Rule}.
115+
* @param name the name to initialize the builder with.
116+
* @param configuration the configuration to initialize the builder with.
117+
* @param visibility the {@link Visibility} to initialize the builder with.
118+
* @return The new {@link RuleBuilder}.
119+
*/
85120
public static RuleBuilder create(RuleTemplate template, String uid, @Nullable String name,
86121
Configuration configuration, Visibility visibility) {
87122
return create(uid).withActions(template.getActions()).withConditions(template.getConditions())

‎bundles/org.openhab.core.config.core/src/main/java/org/openhab/core/config/core/ConfigDescriptionParameter.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ public Type getType() {
286286
}
287287

288288
/**
289-
* @return true if the value is required, otherwise false.
289+
* @return {@code true} if the value is read-only, {@code false} otherwise.
290290
*/
291291
public boolean isReadOnly() {
292292
return readOnly;

‎bundles/org.openhab.core.model.script/.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
/model
22
/plugin.xml_gen
3+
/src.moved/

‎bundles/org.openhab.core.model.script/src/org/openhab/core/model/script/scoping/StateAndCommandProvider.java‎

Lines changed: 38 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
*/
1313
package org.openhab.core.model.script.scoping;
1414

15+
import java.util.Collections;
1516
import java.util.HashSet;
1617
import java.util.Set;
1718

@@ -38,53 +39,53 @@
3839
*/
3940
public class StateAndCommandProvider {
4041

41-
protected static final Set<Command> COMMANDS = new HashSet<>();
42-
protected static final Set<State> STATES = new HashSet<>();
43-
protected static final Set<Type> TYPES = new HashSet<>();
42+
protected static final Set<Command> COMMANDS = Set.of( //
43+
IncreaseDecreaseType.DECREASE, //
44+
IncreaseDecreaseType.INCREASE, //
45+
NextPreviousType.NEXT, //
46+
NextPreviousType.PREVIOUS, //
47+
OnOffType.OFF, //
48+
OnOffType.ON, //
49+
PlayPauseType.PLAY, //
50+
PlayPauseType.PAUSE, //
51+
RefreshType.REFRESH, //
52+
RewindFastforwardType.FASTFORWARD, //
53+
RewindFastforwardType.REWIND, //
54+
StopMoveType.MOVE, //
55+
StopMoveType.STOP, //
56+
UpDownType.DOWN, //
57+
UpDownType.UP);
58+
protected static final Set<State> STATES = Set.of( //
59+
OnOffType.OFF, //
60+
OnOffType.ON, //
61+
OpenClosedType.CLOSED, //
62+
OpenClosedType.OPEN, //
63+
PlayPauseType.PAUSE, //
64+
PlayPauseType.PLAY, //
65+
RewindFastforwardType.FASTFORWARD, //
66+
RewindFastforwardType.REWIND, //
67+
UnDefType.NULL, //
68+
UnDefType.UNDEF, //
69+
UpDownType.DOWN, //
70+
UpDownType.UP);
71+
protected static final Set<Type> TYPES;
4472

4573
static {
46-
COMMANDS.add(OnOffType.ON);
47-
COMMANDS.add(OnOffType.OFF);
48-
COMMANDS.add(UpDownType.UP);
49-
COMMANDS.add(UpDownType.DOWN);
50-
COMMANDS.add(IncreaseDecreaseType.INCREASE);
51-
COMMANDS.add(IncreaseDecreaseType.DECREASE);
52-
COMMANDS.add(StopMoveType.STOP);
53-
COMMANDS.add(StopMoveType.MOVE);
54-
COMMANDS.add(PlayPauseType.PLAY);
55-
COMMANDS.add(PlayPauseType.PAUSE);
56-
COMMANDS.add(NextPreviousType.NEXT);
57-
COMMANDS.add(NextPreviousType.PREVIOUS);
58-
COMMANDS.add(RewindFastforwardType.REWIND);
59-
COMMANDS.add(RewindFastforwardType.FASTFORWARD);
60-
COMMANDS.add(RefreshType.REFRESH);
61-
62-
STATES.add(UnDefType.UNDEF);
63-
STATES.add(UnDefType.NULL);
64-
STATES.add(OnOffType.ON);
65-
STATES.add(OnOffType.OFF);
66-
STATES.add(UpDownType.UP);
67-
STATES.add(UpDownType.DOWN);
68-
STATES.add(OpenClosedType.OPEN);
69-
STATES.add(OpenClosedType.CLOSED);
70-
STATES.add(PlayPauseType.PLAY);
71-
STATES.add(PlayPauseType.PAUSE);
72-
STATES.add(RewindFastforwardType.REWIND);
73-
STATES.add(RewindFastforwardType.FASTFORWARD);
74-
75-
TYPES.addAll(COMMANDS);
76-
TYPES.addAll(STATES);
74+
Set<Type> types = new HashSet<>();
75+
types.addAll(COMMANDS);
76+
types.addAll(STATES);
77+
TYPES = Collections.unmodifiableSet(types);
7778
}
7879

79-
static public Iterable<Type> getAllTypes() {
80+
public static Iterable<Type> getAllTypes() {
8081
return TYPES;
8182
}
8283

83-
static public Iterable<Command> getAllCommands() {
84+
public static Iterable<Command> getAllCommands() {
8485
return COMMANDS;
8586
}
8687

87-
static public Iterable<State> getAllStates() {
88+
public static Iterable<State> getAllStates() {
8889
return STATES;
8990
}
9091
}

‎bundles/org.openhab.core.model.yaml/pom.xml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,5 +40,10 @@
4040
<artifactId>org.openhab.core.automation</artifactId>
4141
<version>${project.version}</version>
4242
</dependency>
43+
<dependency>
44+
<groupId>org.openhab.core.bundles</groupId>
45+
<artifactId>org.openhab.core.automation.module.script.rulesupport</artifactId>
46+
<version>${project.version}</version>
47+
</dependency>
4348
</dependencies>
4449
</project>

‎bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/config/YamlConfigDescriptionParameterDTO.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,11 @@ public YamlConfigDescriptionParameterDTO(@NonNull ConfigDescriptionParameter par
9696
}
9797
this.options = optionDtos;
9898
}
99-
List<@NonNull FilterCriteria> filterCriterias = parameter.getFilterCriteria();
100-
if (!filterCriterias.isEmpty()) {
99+
List<@NonNull FilterCriteria> filterCriteria = parameter.getFilterCriteria();
100+
if (!filterCriteria.isEmpty()) {
101101
List<FilterCriteriaDTO> filterCriteriaDtos = new ArrayList<>(filterCriteria.size());
102-
for (FilterCriteria filterCriteria : filterCriterias) {
103-
filterCriteriaDtos.add(new FilterCriteriaDTO(filterCriteria.getName(), filterCriteria.getValue()));
102+
for (FilterCriteria filterCriterion : filterCriteria) {
103+
filterCriteriaDtos.add(new FilterCriteriaDTO(filterCriterion.getName(), filterCriterion.getValue()));
104104
}
105105
this.filterCriteria = filterCriteriaDtos;
106106
}

0 commit comments

Comments
 (0)