Skip to content

Commit

Permalink
[automation] Ensure unique module IDs for overloaded @RuleAction me…
Browse files Browse the repository at this point in the history 
…thods (#4441)

* [automation] Handle overloaded `@RuleAction`s - Append signature hash to avoid duplicate module ids.
* [automation] ThingActionsResource: Provide action visibility

Signed-off-by: Florian Hotze <[email protected]>
  • Loading branch information
@florian-h05
florian-h05 authored Nov 10, 2024
1 parent 3bc9ae3 commit 0d031a3
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 24 deletions.
43 changes: 26 additions & 17 deletions ...on.rest/src/main/java/org/openhab/core/automation/rest/internal/ThingActionsResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,11 @@
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.auth.Role;
import org.openhab.core.automation.Action;
import org.openhab.core.automation.Visibility;
import org.openhab.core.automation.annotation.RuleAction;
import org.openhab.core.automation.handler.ActionHandler;
import org.openhab.core.automation.handler.ModuleHandlerFactory;
import org.openhab.core.automation.module.provider.AnnotationActionModuleTypeHelper;
import org.openhab.core.automation.type.ActionType;
import org.openhab.core.automation.type.Input;
import org.openhab.core.automation.type.ModuleTypeRegistry;
Expand Down Expand Up @@ -97,16 +99,19 @@ public class ThingActionsResource implements RESTResource {
private final LocaleService localeService;
private final ModuleTypeRegistry moduleTypeRegistry;
private final ActionInputsHelper actionInputsHelper;
private final AnnotationActionModuleTypeHelper annotationActionModuleTypeHelper;

Map<ThingUID, Map<String, ThingActions>> thingActionsMap = new ConcurrentHashMap<>();
Map<ThingUID, Map<String, List<String>>> thingActionsMap = new ConcurrentHashMap<>();
private List<ModuleHandlerFactory> moduleHandlerFactories = new ArrayList<>();

@Activate
public ThingActionsResource(@Reference LocaleService localeService,
@Reference ModuleTypeRegistry moduleTypeRegistry, @Reference ActionInputsHelper actionInputsHelper) {
@Reference ModuleTypeRegistry moduleTypeRegistry, @Reference ActionInputsHelper actionInputsHelper,
@Reference AnnotationActionModuleTypeHelper annotationActionModuleTypeHelper) {
this.localeService = localeService;
this.moduleTypeRegistry = moduleTypeRegistry;
this.actionInputsHelper = actionInputsHelper;
this.annotationActionModuleTypeHelper = annotationActionModuleTypeHelper;
}

@Reference(policy = ReferencePolicy.DYNAMIC, cardinality = ReferenceCardinality.MULTIPLE)
Expand All @@ -115,7 +120,18 @@ public void addThingActions(ThingActions thingActions) {
String scope = getScope(thingActions);
if (handler != null && scope != null) {
ThingUID thingUID = handler.getThing().getUID();
thingActionsMap.computeIfAbsent(thingUID, thingUid -> new ConcurrentHashMap<>()).put(scope, thingActions);
Method[] methods = thingActions.getClass().getDeclaredMethods();
List<String> actionUIDs = new ArrayList<>();
for (Method method : methods) {
if (!method.isAnnotationPresent(RuleAction.class)) {
continue;
}
actionUIDs.add(annotationActionModuleTypeHelper.getModuleIdFromMethod(scope, method));
}
if (actionUIDs.isEmpty()) {
return;
}
thingActionsMap.computeIfAbsent(thingUID, thingUid -> new ConcurrentHashMap<>()).put(scope, actionUIDs);
}
}

Expand All @@ -124,7 +140,7 @@ public void removeThingActions(ThingActions thingActions) {
String scope = getScope(thingActions);
if (handler != null && scope != null) {
ThingUID thingUID = handler.getThing().getUID();
Map<String, ThingActions> actionMap = thingActionsMap.get(thingUID);
Map<String, List<String>> actionMap = thingActionsMap.get(thingUID);
if (actionMap != null) {
actionMap.remove(scope);
if (actionMap.isEmpty()) {
Expand Down Expand Up @@ -156,24 +172,15 @@ public Response getActions(@PathParam("thingUID") @Parameter(description = "thin
ThingUID aThingUID = new ThingUID(thingUID);

List<ThingActionDTO> actions = new ArrayList<>();
Map<String, ThingActions> thingActionsMap = this.thingActionsMap.get(aThingUID);
Map<String, List<String>> thingActionsMap = this.thingActionsMap.get(aThingUID);
if (thingActionsMap == null) {
return Response.status(Response.Status.NOT_FOUND).build();
}

// inspect ThingActions
for (Map.Entry<String, ThingActions> thingActionsEntry : thingActionsMap.entrySet()) {
ThingActions thingActions = thingActionsEntry.getValue();
Method[] methods = thingActions.getClass().getDeclaredMethods();
for (Method method : methods) {
RuleAction ruleAction = method.getAnnotation(RuleAction.class);

if (ruleAction == null) {
continue;
}

String actionUid = thingActionsEntry.getKey() + "." + method.getName();
ActionType actionType = (ActionType) moduleTypeRegistry.get(actionUid, locale);
for (Map.Entry<String, List<String>> thingActionsEntry : thingActionsMap.entrySet()) {
for (String actionUID : thingActionsEntry.getValue()) {
ActionType actionType = (ActionType) moduleTypeRegistry.get(actionUID, locale);
if (actionType == null) {
continue;
}
Expand All @@ -200,6 +207,7 @@ public Response getActions(@PathParam("thingUID") @Parameter(description = "thin
actionDTO.inputConfigDescriptions = inputParameters == null ? null
: ConfigDescriptionDTOMapper.mapParameters(inputParameters);
actionDTO.outputs = actionType.getOutputs();
actionDTO.visibility = actionType.getVisibility();
actions.add(actionDTO);
}
}
Expand Down Expand Up @@ -268,6 +276,7 @@ private static class ThingActionDTO {

public @Nullable String label;
public @Nullable String description;
public @Nullable Visibility visibility;

public List<Input> inputs = new ArrayList<>();

Expand Down
22 changes: 20 additions & 2 deletions ...in/java/org/openhab/core/automation/module/provider/AnnotationActionModuleTypeHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
import java.lang.annotation.Annotation;
import java.lang.reflect.Method;
import java.lang.reflect.Parameter;
import java.math.BigInteger;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
Expand Down Expand Up @@ -57,7 +60,8 @@
* Helper methods for {@link AnnotatedActions} {@link ModuleTypeProvider}
*
* @author Stefan Triller - Initial contribution
* @author Florian Hotze - Added configuration description parameters for thing modules
* @author Florian Hotze - Added configuration description parameters for thing modules, Added method signature hash to
* module ID in case of method overloads
* @author Laurent Garnier - Converted into a an OSGi component
*/
@NonNullByDefault
Expand Down Expand Up @@ -96,7 +100,7 @@ public Collection<ModuleInformation> parseAnnotations(String name, Object action
List<Output> outputs = getOutputsFromAction(method);

RuleAction ruleAction = method.getAnnotation(RuleAction.class);
String uid = name + "." + method.getName();
String uid = getModuleIdFromMethod(name, method);
Set<String> tags = new HashSet<>(Arrays.asList(ruleAction.tags()));

ModuleInformation mi = new ModuleInformation(uid, actionProvider, method);
Expand All @@ -113,6 +117,20 @@ public Collection<ModuleInformation> parseAnnotations(String name, Object action
return moduleInformation;
}

public String getModuleIdFromMethod(String actionScope, Method method) {
String uid = actionScope + "." + method.getName() + "#";
MessageDigest md5 = null;
try {
md5 = MessageDigest.getInstance("MD5");
} catch (NoSuchAlgorithmException e) {
throw new RuntimeException(e);
}
for (Class<?> parameter : method.getParameterTypes()) {
md5.update(parameter.getName().getBytes());
}
return uid + String.format("%032x", new BigInteger(1, md5.digest()));
}

private List<Input> getInputsFromAction(Method method) {
List<Input> inputs = new ArrayList<>();

Expand Down
30 changes: 26 additions & 4 deletions ...nhab/core/automation/internal/module/provider/AnnotationActionModuleTypeProviderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.when;

import java.lang.reflect.Method;
import java.math.BigInteger;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
Expand All @@ -33,6 +38,7 @@
import org.openhab.core.automation.Visibility;
import org.openhab.core.automation.annotation.ActionInput;
import org.openhab.core.automation.annotation.ActionOutput;
import org.openhab.core.automation.annotation.ActionOutputs;
import org.openhab.core.automation.annotation.ActionScope;
import org.openhab.core.automation.annotation.RuleAction;
import org.openhab.core.automation.module.provider.AnnotationActionModuleTypeHelper;
Expand All @@ -55,7 +61,22 @@
@NonNullByDefault
public class AnnotationActionModuleTypeProviderTest extends JavaTest {

private static final String TEST_ACTION_TYPE_ID = "binding.test.testMethod";
private static final String TEST_ACTION_SIGNATURE_HASH;
static {
MessageDigest md5 = null;
try {
md5 = MessageDigest.getInstance("MD5");
} catch (NoSuchAlgorithmException e) {
throw new RuntimeException(e);
}
Method method = Arrays.stream(TestActionProvider.class.getDeclaredMethods())
.filter(m -> m.getName().equals("testMethod")).findFirst().orElseThrow();
for (Class<?> parameter : method.getParameterTypes()) {
md5.update(parameter.getName().getBytes());
}
TEST_ACTION_SIGNATURE_HASH = String.format("%032x", new BigInteger(1, md5.digest()));
}
private static final String TEST_ACTION_TYPE_ID = "binding.test.testMethod#" + TEST_ACTION_SIGNATURE_HASH;
private static final String ACTION_LABEL = "Test Label";
private static final String ACTION_DESCRIPTION = "My Description";

Expand Down Expand Up @@ -195,9 +216,10 @@ private static class TestActionProvider implements AnnotatedActions {

@RuleAction(label = ACTION_LABEL, description = ACTION_DESCRIPTION, visibility = Visibility.HIDDEN, tags = {
"tag1", "tag2" })
public @ActionOutput(name = ACTION_OUTPUT1, type = ACTION_OUTPUT1_TYPE, description = ACTION_OUTPUT1_DESCRIPTION, label = ACTION_OUTPUT1_LABEL, defaultValue = ACTION_OUTPUT1_DEFAULT_VALUE, reference = ACTION_OUTPUT1_REFERENCE, tags = {
"tagOut11",
"tagOut12" }) @ActionOutput(name = ACTION_OUTPUT2, type = ACTION_OUTPUT2_TYPE) Map<String, Object> testMethod(
public @ActionOutputs({
@ActionOutput(name = ACTION_OUTPUT1, type = ACTION_OUTPUT1_TYPE, description = ACTION_OUTPUT1_DESCRIPTION, label = ACTION_OUTPUT1_LABEL, defaultValue = ACTION_OUTPUT1_DEFAULT_VALUE, reference = ACTION_OUTPUT1_REFERENCE, tags = {
"tagOut11", "tagOut12" }),
@ActionOutput(name = ACTION_OUTPUT2, type = ACTION_OUTPUT2_TYPE) }) Map<String, Object> testMethod(
@ActionInput(name = ACTION_INPUT1, label = ACTION_INPUT1_LABEL, defaultValue = ACTION_INPUT1_DEFAULT_VALUE, description = ACTION_INPUT1_DESCRIPTION, reference = ACTION_INPUT1_REFERENCE, required = true, type = "Item", tags = {
"tagIn11", "tagIn12" }) String input1,
@ActionInput(name = ACTION_INPUT2) String input2) {
Expand Down
22 changes: 21 additions & 1 deletion .../org/openhab/core/automation/thingsupport/AnnotatedThingActionModuleTypeProviderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.when;

import java.lang.reflect.Method;
import java.math.BigInteger;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
Expand Down Expand Up @@ -60,7 +65,22 @@ public class AnnotatedThingActionModuleTypeProviderTest extends JavaTest {

private static final ThingTypeUID TEST_THING_TYPE_UID = new ThingTypeUID("binding", "thing-type");

private static final String TEST_ACTION_TYPE_ID = "test.testMethod";
private static final String TEST_ACTION_SIGNATURE_HASH;
static {
MessageDigest md5 = null;
try {
md5 = MessageDigest.getInstance("MD5");
} catch (NoSuchAlgorithmException e) {
throw new RuntimeException(e);
}
Method method = Arrays.stream(TestThingActionProvider.class.getDeclaredMethods())
.filter(m -> m.getName().equals("testMethod")).findFirst().orElseThrow();
for (Class<?> parameter : method.getParameterTypes()) {
md5.update(parameter.getName().getBytes());
}
TEST_ACTION_SIGNATURE_HASH = String.format("%032x", new BigInteger(1, md5.digest()));
}
private static final String TEST_ACTION_TYPE_ID = "test.testMethod#" + TEST_ACTION_SIGNATURE_HASH;
private static final String ACTION_LABEL = "Test Label";
private static final String ACTION_DESCRIPTION = "My Description";

Expand Down

0 comments on commit 0d031a3

Please sign in to comment.