You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@unomi.apache.org by GitBox <gi...@apache.org> on 2021/03/18 14:24:00 UTC
[GitHub] [unomi] Taybou opened a new pull request #265: UNOMI-446 improve increment action and make it generic
Taybou opened a new pull request #265:
URL: https://github.com/apache/unomi/pull/265
----
**Please** following this checklist to help us incorporate your contribution quickly and easily:
- [X] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/UNOMI) filed
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without pulling in other changes.
- [X] Format the pull request title like `[UNOMI-XXX] - Title of the pull request`
- [ ] Provide integration tests for your changes, especially if you are changing the behavior of existing code or adding
significant new parts of code.
- [X] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Copy the description to the related JIRA issue
- [X] Run `mvn clean install -P integration-tests` to make sure basic checks pass. A more thorough check will be
performed on your pull request automatically.
Trivial changes like typos do not require a JIRA issue (javadoc, project build changes, small doc changes, comments...).
If this is your first contribution, you have to read the [Contribution Guidelines](https://unomi.apache.org/contribute.html)
If your pull request is about ~20 lines of code you don't need to sign an [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf)
if you are unsure please ask on the developers list.
To make clear that you license your contribution under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [X] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [unomi] jkevan edited a comment on pull request #265: UNOMI-446 improve increment action and make it generic
Posted by GitBox <gi...@apache.org>.
jkevan edited a comment on pull request #265:
URL: https://github.com/apache/unomi/pull/265#issuecomment-804209534
### Small update on what we discussed about in the call:
- Do not touch the unomi IncrementInterestAction. This action have his own logic and cannot be generify with property incrementation, also the interest incrementation from JExperience is not using this action, it's actually using a script and the setPropertyAction.
- Create a new Action: IncrementPropertyAction:
- propertyName: to be able to increment or create a single prop (properties.pageViewCount)
- rootPropertyName: to be able to increment all props under this one: (ex: profile.properties.interest)
- incrementWithValues: to be able to increment properties with the values coming from the event. (ex: event.properties.interest)
- storeInSession: to be able to target the profile or the session.
### Real exemple coming from JEXperience:
**the increment page count:**
```
"actions": [
{
"parameterValues": {
"setPropertyName": "properties.pageViewCount",
"setPropertyValue": "script::r = profile.properties['pageViewCount']; if (r == null) { profile.properties['pageViewCount'] = []; profile.properties.pageViewCount = [event.scope : 1] } else { if (r[event.scope] != null) { r[event.scope] = r[event.scope] + 1 } else { r[event.scope] = 1 }} r",
"storeInSession": false
},
"type": "setPropertyAction"
}
]
```
We should be able to do it with this kind of conf:
**type: incrementPropertyAction**
**paramerterValues:**
- **propertyName: properties.pageViewCount**
- **storeInSession: false**
**The increment interest from jexperience:**
```
"actions": [
{
"parameterValues": {
"setPropertyName": "properties.interests",
"setPropertyValue": "script::r = profile.properties['interests']; foreach(interest : event.target.properties['interests'].entrySet()) { if (r == null) { r = [interest.key: interest.value] } else if (r[interest.key] != null) { r[interest.key] = r[interest.key] + interest.value } else { r[interest.key] = interest.value } } r",
"storeInSession": false
},
"type": "setPropertyAction"
}
]
```
We should be able to replace it, with:
**type:incrementPropertyAction**
**paramerterValues:**
- **rootPropertyName: properties.interest**
- **incrementWithEventValues: target.properties.interest**
- **storeInSession: false**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [unomi] sergehuber commented on a change in pull request #265: UNOMI-446 improve increment action and make it generic
Posted by GitBox <gi...@apache.org>.
sergehuber commented on a change in pull request #265:
URL: https://github.com/apache/unomi/pull/265#discussion_r598728046
##########
File path: plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/IncrementPropertyAction.java
##########
@@ -16,56 +16,88 @@
*/
package org.apache.unomi.plugins.baseplugin.actions;
+import java.lang.reflect.InvocationTargetException;
import java.util.Date;
import java.util.HashMap;
import java.util.Map;
+import org.apache.commons.beanutils.PropertyUtils;
import org.apache.unomi.api.Event;
import org.apache.unomi.api.Profile;
import org.apache.unomi.api.actions.Action;
import org.apache.unomi.api.actions.ActionExecutor;
import org.apache.unomi.api.services.EventService;
import org.apache.unomi.api.services.TopicService;
+import org.apache.unomi.persistence.spi.PropertyHelper;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-public class IncrementInterestAction implements ActionExecutor {
+public class IncrementPropertyAction implements ActionExecutor {
- private static final Logger LOG = LoggerFactory.getLogger(IncrementInterestAction.class.getName());
+ private static final Logger LOG = LoggerFactory.getLogger(IncrementPropertyAction.class.getName());
private static final String EVENT_INTERESTS_PROPERTY = "interests";
-
private static final String ACTION_INTERESTS_PROPERTY = "eventInterestProperty";
private TopicService topicService;
-
private EventService eventService;
-
private Double interestsMinValue;
-
private Double interestsMaxValue;
-
private Double interestsDividerValue;
@Override
@SuppressWarnings("unchecked")
public int execute(final Action action, final Event event) {
- Map<String, Double> interestsAsMap = (Map<String, Double>) action.getParameterValues().get( ACTION_INTERESTS_PROPERTY );
+ final Profile profile = event.getProfile();
- if ( interestsAsMap == null ) {
- interestsAsMap = (Map<String, Double>) event.getProperty( EVENT_INTERESTS_PROPERTY );
+ Object propertyValue;
+ String propertyName = (String) action.getParameterValues().get("propertyName");
+ String parentPropertyName = propertyName.split("\\.")[0];
Review comment:
What happens if no parent is provided. Isn't the parentPropertyName and the propertyName the same in that case?P
##########
File path: plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/IncrementPropertyAction.java
##########
@@ -16,56 +16,88 @@
*/
package org.apache.unomi.plugins.baseplugin.actions;
+import java.lang.reflect.InvocationTargetException;
import java.util.Date;
import java.util.HashMap;
import java.util.Map;
+import org.apache.commons.beanutils.PropertyUtils;
import org.apache.unomi.api.Event;
import org.apache.unomi.api.Profile;
import org.apache.unomi.api.actions.Action;
import org.apache.unomi.api.actions.ActionExecutor;
import org.apache.unomi.api.services.EventService;
import org.apache.unomi.api.services.TopicService;
+import org.apache.unomi.persistence.spi.PropertyHelper;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-public class IncrementInterestAction implements ActionExecutor {
+public class IncrementPropertyAction implements ActionExecutor {
- private static final Logger LOG = LoggerFactory.getLogger(IncrementInterestAction.class.getName());
+ private static final Logger LOG = LoggerFactory.getLogger(IncrementPropertyAction.class.getName());
private static final String EVENT_INTERESTS_PROPERTY = "interests";
-
private static final String ACTION_INTERESTS_PROPERTY = "eventInterestProperty";
private TopicService topicService;
-
private EventService eventService;
-
private Double interestsMinValue;
-
private Double interestsMaxValue;
-
private Double interestsDividerValue;
@Override
@SuppressWarnings("unchecked")
public int execute(final Action action, final Event event) {
- Map<String, Double> interestsAsMap = (Map<String, Double>) action.getParameterValues().get( ACTION_INTERESTS_PROPERTY );
+ final Profile profile = event.getProfile();
- if ( interestsAsMap == null ) {
- interestsAsMap = (Map<String, Double>) event.getProperty( EVENT_INTERESTS_PROPERTY );
+ Object propertyValue;
+ String propertyName = (String) action.getParameterValues().get("propertyName");
+ String parentPropertyName = propertyName.split("\\.")[0];
- if (interestsAsMap == null) {
- return EventService.NO_CHANGE;
+ try {
+ if (EVENT_INTERESTS_PROPERTY.equals(propertyName)) {
+ propertyValue = incrementInterests(action, event, profile);
Review comment:
After reflection I wonder if we should split this action into 2 separate actions: IncrementInterests and IncrementProperty, because their logic is quite different.
##########
File path: services/src/main/java/org/apache/unomi/services/actions/ActionExecutorDispatcher.java
##########
@@ -124,13 +126,21 @@ public Action getContextualAction(Action action, Event event) {
if (value instanceof String) {
String s = (String) value;
try {
- // check if we have special values
- if (s.contains(VALUE_NAME_SEPARATOR)) {
- final String valueType = StringUtils.substringBefore(s, VALUE_NAME_SEPARATOR);
- final String valueAsString = StringUtils.substringAfter(s, VALUE_NAME_SEPARATOR);
- final ValueExtractor extractor = valueExtractors.get(valueType);
- if (extractor != null) {
- value = extractor.extract(valueAsString, event);
+ if (s.contains(PLACEHOLDER_PREFIX)) {
+ while (s.contains(PLACEHOLDER_PREFIX)) {
+ String substring = s.substring(s.indexOf(PLACEHOLDER_PREFIX) + 2, s.indexOf(PLACEHOLDER_SUFFIX));
Review comment:
What happens if the PLACEHOLDER_SUFFIX is not found, does it raise an exception because indexOf is < 0 ?
##########
File path: plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/IncrementPropertyAction.java
##########
@@ -16,56 +16,88 @@
*/
package org.apache.unomi.plugins.baseplugin.actions;
+import java.lang.reflect.InvocationTargetException;
import java.util.Date;
import java.util.HashMap;
import java.util.Map;
+import org.apache.commons.beanutils.PropertyUtils;
import org.apache.unomi.api.Event;
import org.apache.unomi.api.Profile;
import org.apache.unomi.api.actions.Action;
import org.apache.unomi.api.actions.ActionExecutor;
import org.apache.unomi.api.services.EventService;
import org.apache.unomi.api.services.TopicService;
+import org.apache.unomi.persistence.spi.PropertyHelper;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-public class IncrementInterestAction implements ActionExecutor {
+public class IncrementPropertyAction implements ActionExecutor {
- private static final Logger LOG = LoggerFactory.getLogger(IncrementInterestAction.class.getName());
+ private static final Logger LOG = LoggerFactory.getLogger(IncrementPropertyAction.class.getName());
private static final String EVENT_INTERESTS_PROPERTY = "interests";
-
private static final String ACTION_INTERESTS_PROPERTY = "eventInterestProperty";
private TopicService topicService;
-
private EventService eventService;
-
private Double interestsMinValue;
-
private Double interestsMaxValue;
-
private Double interestsDividerValue;
@Override
@SuppressWarnings("unchecked")
public int execute(final Action action, final Event event) {
- Map<String, Double> interestsAsMap = (Map<String, Double>) action.getParameterValues().get( ACTION_INTERESTS_PROPERTY );
+ final Profile profile = event.getProfile();
- if ( interestsAsMap == null ) {
- interestsAsMap = (Map<String, Double>) event.getProperty( EVENT_INTERESTS_PROPERTY );
+ Object propertyValue;
+ String propertyName = (String) action.getParameterValues().get("propertyName");
+ String parentPropertyName = propertyName.split("\\.")[0];
Review comment:
Maybe it's that's normal, maybe we could rename it to rootPropertyName?
##########
File path: plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/IncrementPropertyAction.java
##########
@@ -16,56 +16,88 @@
*/
package org.apache.unomi.plugins.baseplugin.actions;
+import java.lang.reflect.InvocationTargetException;
import java.util.Date;
import java.util.HashMap;
import java.util.Map;
+import org.apache.commons.beanutils.PropertyUtils;
import org.apache.unomi.api.Event;
import org.apache.unomi.api.Profile;
import org.apache.unomi.api.actions.Action;
import org.apache.unomi.api.actions.ActionExecutor;
import org.apache.unomi.api.services.EventService;
import org.apache.unomi.api.services.TopicService;
+import org.apache.unomi.persistence.spi.PropertyHelper;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-public class IncrementInterestAction implements ActionExecutor {
+public class IncrementPropertyAction implements ActionExecutor {
- private static final Logger LOG = LoggerFactory.getLogger(IncrementInterestAction.class.getName());
+ private static final Logger LOG = LoggerFactory.getLogger(IncrementPropertyAction.class.getName());
private static final String EVENT_INTERESTS_PROPERTY = "interests";
-
private static final String ACTION_INTERESTS_PROPERTY = "eventInterestProperty";
private TopicService topicService;
-
private EventService eventService;
-
private Double interestsMinValue;
-
private Double interestsMaxValue;
-
private Double interestsDividerValue;
@Override
@SuppressWarnings("unchecked")
public int execute(final Action action, final Event event) {
- Map<String, Double> interestsAsMap = (Map<String, Double>) action.getParameterValues().get( ACTION_INTERESTS_PROPERTY );
+ final Profile profile = event.getProfile();
- if ( interestsAsMap == null ) {
- interestsAsMap = (Map<String, Double>) event.getProperty( EVENT_INTERESTS_PROPERTY );
+ Object propertyValue;
+ String propertyName = (String) action.getParameterValues().get("propertyName");
+ String parentPropertyName = propertyName.split("\\.")[0];
- if (interestsAsMap == null) {
- return EventService.NO_CHANGE;
+ try {
+ if (EVENT_INTERESTS_PROPERTY.equals(propertyName)) {
+ propertyValue = incrementInterests(action, event, profile);
Review comment:
I think we should rename this method to processIncrementInterestEvent
##########
File path: itests/src/test/java/org/apache/unomi/itests/IncrementPropertyIT.java
##########
@@ -0,0 +1,305 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+package org.apache.unomi.itests;
+
+import java.util.*;
+
+import javax.inject.Inject;
+
+import org.apache.unomi.api.CustomItem;
+import org.apache.unomi.api.Event;
+import org.apache.unomi.api.Metadata;
+import org.apache.unomi.api.Profile;
+import org.apache.unomi.api.Topic;
+import org.apache.unomi.api.actions.Action;
+import org.apache.unomi.api.conditions.Condition;
+import org.apache.unomi.api.rules.Rule;
+import org.apache.unomi.api.services.DefinitionsService;
+import org.apache.unomi.api.services.EventService;
+import org.apache.unomi.api.services.ProfileService;
+import org.apache.unomi.api.services.RulesService;
+import org.apache.unomi.api.services.TopicService;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.ops4j.pax.exam.junit.PaxExam;
+import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy;
+import org.ops4j.pax.exam.spi.reactors.PerSuite;
+import org.ops4j.pax.exam.util.Filter;
+
+import static org.apache.unomi.itests.BasicIT.ITEM_TYPE_PAGE;
+
+@RunWith(PaxExam.class)
+@ExamReactorStrategy(PerSuite.class)
+public class IncrementPropertyIT extends BaseIT {
+ private Profile profile;
+ private Topic topic;
+ private Rule rule;
+
+ @Inject
+ @Filter(timeout = 600000)
+ protected ProfileService profileService;
+
+ @Inject
+ @Filter(timeout = 600000)
+ protected EventService eventService;
+
+ @Inject
+ @Filter(timeout = 600000)
+ protected TopicService topicService;
+
+ @Inject
+ @Filter(timeout = 600000)
+ protected RulesService rulesService;
+
+ @Inject
+ @Filter(timeout = 600000)
+ protected DefinitionsService definitionsService;
+
+ @Before
+ public void setup() throws Exception {
+ profile = createProfile();
+ topic = createTopic("topicId");
+ rule = new Rule();
+ }
+
+ @After
+ public void tearDown() {
+ rulesService.removeRule(rule.getItemId());
+ topicService.delete(topic.getItemId());
+ profileService.delete(profile.getItemId(), false);
+ }
+
+ @Test
+ public void testIncrementInterests() throws InterruptedException {
+ final Map<String, Double> interestsAsMap = new HashMap<>();
+ interestsAsMap.put(topic.getTopicId(), 50.0);
+ interestsAsMap.put("unknown", 10.0);
+
+ final Event event = createEvent(profile, interestsAsMap);
+
+ int eventCode = eventService.send(event);
+
+ if (eventCode == EventService.PROFILE_UPDATED) {
+ Profile updatedProfile = profileService.save(event.getProfile());
+
+ refreshPersistence();
+
+ Map<String, Double> interests = (Map<String, Double>) updatedProfile.getProperty("interests");
+
+ Assert.assertEquals(0.5, interests.get(topic.getTopicId()), 0.0);
+ Assert.assertFalse(interests.containsKey("unknown"));
+ } else {
+ throw new IllegalStateException("Profile was not updated");
+ }
+ }
+
+ @Test
+ public void testIncrementInterestsByAction() throws InterruptedException {
+ Action incrementPropertyAction = new Action(definitionsService.getActionType("incrementPropertyAction"));
+ incrementPropertyAction.setParameter("propertyName", "interests");
+ incrementPropertyAction.setParameter("eventInterestProperty", "eventProperty::target.properties.interests");
+
+ Condition condition = createCondition();
+ Metadata metadata = createMetadata();
+
+ List<Action> actions = new ArrayList<>();
+ actions.add(incrementPropertyAction);
+
+ rule.setCondition(condition);
+ rule.setActions(actions);
+ rule.setMetadata(metadata);
+
+ rulesService.setRule(rule);
+ refreshPersistence();
+
+ keepTrying("Failed waiting for the creation of the rule for the IncrementPropertyIT test",
+ () -> rulesService.getRule(rule.getItemId()), Objects::nonNull, 1000, 100);
+
+ Map<String, Double> interestsAsMap = new HashMap<>();
+ interestsAsMap.put(topic.getTopicId(), 50.0);
+ interestsAsMap.put("unknown", 10.0);
+
+ Map<String, Object> properties = new HashMap<>();
+
+ properties.put("interests", interestsAsMap);
+
+ CustomItem item = new CustomItem("page", ITEM_TYPE_PAGE);
+ item.setProperties(properties);
+
+ Event event = new Event("view", null, profile, null, null, item, new Date());
+ event.setPersistent(false);
+
+ int eventCode = eventService.send(event);
+ refreshPersistence();
+
+ if (eventCode == EventService.PROFILE_UPDATED) {
+ Profile updatedProfile = profileService.save(event.getProfile());
+
+ refreshPersistence();
+
+ Map<String, Double> interests = (Map<String, Double>) updatedProfile.getProperty("interests");
+
+ Assert.assertEquals(0.5, interests.get(topic.getTopicId()), 0.0);
+ Assert.assertFalse(interests.containsKey("unknown"));
+ } else {
+ throw new IllegalStateException("Profile was not updated");
+ }
+ }
+
+ @Test
+ public void testIncrementPropertyByAction() throws InterruptedException {
+ Action incrementPropertyAction = new Action(definitionsService.getActionType("incrementPropertyAction"));
+ incrementPropertyAction.setParameter("propertyName", "pageView.acme");
+
+ Condition condition = createCondition();
+ Metadata metadata = createMetadata();
+
+ List<Action> actions = new ArrayList<>();
+ actions.add(incrementPropertyAction);
+
+ rule.setCondition(condition);
+ rule.setActions(actions);
+ rule.setMetadata(metadata);
+ rulesService.setRule(rule);
+ refreshPersistence();
+
+ keepTrying("Failed waiting for the creation of the rule for the IncrementPropertyIT test",
+ () -> rulesService.getRule(rule.getItemId()), Objects::nonNull, 1000, 100);
+
+
+ Event event = new Event("view", null, profile, null, null, profile, new Date());
+ event.setPersistent(false);
+
+ int eventCode = eventService.send(event);
+ refreshPersistence();
+
+ if (eventCode == EventService.PROFILE_UPDATED) {
+ Profile updatedProfile = profileService.save(event.getProfile());
+
+ refreshPersistence();
+
+ int value = ((Map<String, Integer>) updatedProfile.getProperty("pageView")).get("acme");
+ Assert.assertEquals(1, value, 0.0);
+ } else {
+ throw new IllegalStateException("Profile was not updated");
+ }
+ }
+
+ @Test
+ public void testIncrementExistingPropertyByAction() throws InterruptedException {
+ Action incrementPropertyAction = new Action(definitionsService.getActionType("incrementPropertyAction"));
Review comment:
Could you please add a test for the case of the ${} marker?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [unomi] jkevan commented on a change in pull request #265: UNOMI-446 improve increment action and make it generic
Posted by GitBox <gi...@apache.org>.
jkevan commented on a change in pull request #265:
URL: https://github.com/apache/unomi/pull/265#discussion_r601660528
##########
File path: plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/IncrementPropertyAction.java
##########
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.unomi.plugins.baseplugin.actions;
+
+import org.apache.commons.beanutils.PropertyUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.unomi.api.*;
+import org.apache.unomi.api.actions.Action;
+import org.apache.unomi.api.actions.ActionExecutor;
+import org.apache.unomi.api.services.EventService;
+import org.apache.unomi.persistence.spi.PropertyHelper;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.InvocationTargetException;
+import java.util.Map;
+
+public class IncrementPropertyAction implements ActionExecutor {
+ private static final Logger logger = LoggerFactory.getLogger(IncrementPropertyAction.class.getName());
+
+ @Override
+ public int execute(final Action action, final Event event) {
+ boolean storeInSession = Boolean.TRUE.equals(action.getParameterValues().get("storeInSession"));
+ if (storeInSession && event.getSession() == null) {
+ return EventService.NO_CHANGE;
+ }
+
+ String propertyName = (String) action.getParameterValues().get("propertyName");
+ Profile profile = event.getProfile();
+ Session session = event.getSession();
+
+ try {
+ Map<String, Object> properties = storeInSession ? session.getProperties() : profile.getProperties();
+ Object propertyValue = getPropertyValue(action, event, propertyName, properties);
+
+ if (PropertyHelper.setProperty(properties, propertyName, propertyValue, "alwaysSet")) {
+ return storeInSession ? EventService.SESSION_UPDATED : EventService.PROFILE_UPDATED;
+ }
+ } catch (IllegalAccessException | NoSuchMethodException | InvocationTargetException e) {
+ logger.warn("Error resolving nested property of object. See debug log level for more information");
+ if (logger.isDebugEnabled()) {
+ logger.error("Error resolving nested property of item: {}", storeInSession ? session : profile, e);
+ }
+ }
+
+ return EventService.NO_CHANGE;
+ }
+
+ private Object getPropertyValue(Action action, Event event, String propertyName, Map<String, Object> properties)
+ throws IllegalAccessException, InvocationTargetException, NoSuchMethodException {
+ String propertyTarget = (String) action.getParameterValues().get("propertyTarget");
+ String rootPropertyName = propertyName.split("\\.")[0];
+ Object propertyValue = 1;
+
+ Object propertyTargetValue = null;
+
+ if (StringUtils.isNotEmpty(propertyTarget)) {
+ propertyTargetValue = PropertyUtils.getNestedProperty(((CustomItem) event.getTarget()).getProperties(), propertyTarget);
+ }
+
+ if (propertyTargetValue != null) {
+ if (propertyTargetValue instanceof Integer) {
+ if (properties.containsKey(rootPropertyName)) {
+ propertyValue = (int) propertyTargetValue + (int) PropertyUtils.getNestedProperty(properties, propertyName);
Review comment:
Careful with cast: (int) PropertyUtils.getNestedProperty(properties, propertyName);
- the PropertyUtils.getNestedProperty(properties, propertyName); can return null ?
- the PropertyUtils.getNestedProperty(properties, propertyName); can return a String, if the prop exist and is a String
##########
File path: plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/IncrementPropertyAction.java
##########
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.unomi.plugins.baseplugin.actions;
+
+import org.apache.commons.beanutils.PropertyUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.unomi.api.*;
+import org.apache.unomi.api.actions.Action;
+import org.apache.unomi.api.actions.ActionExecutor;
+import org.apache.unomi.api.services.EventService;
+import org.apache.unomi.persistence.spi.PropertyHelper;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.InvocationTargetException;
+import java.util.Map;
+
+public class IncrementPropertyAction implements ActionExecutor {
+ private static final Logger logger = LoggerFactory.getLogger(IncrementPropertyAction.class.getName());
+
+ @Override
+ public int execute(final Action action, final Event event) {
+ boolean storeInSession = Boolean.TRUE.equals(action.getParameterValues().get("storeInSession"));
+ if (storeInSession && event.getSession() == null) {
+ return EventService.NO_CHANGE;
+ }
+
+ String propertyName = (String) action.getParameterValues().get("propertyName");
+ Profile profile = event.getProfile();
+ Session session = event.getSession();
+
+ try {
+ Map<String, Object> properties = storeInSession ? session.getProperties() : profile.getProperties();
+ Object propertyValue = getPropertyValue(action, event, propertyName, properties);
+
+ if (PropertyHelper.setProperty(properties, propertyName, propertyValue, "alwaysSet")) {
+ return storeInSession ? EventService.SESSION_UPDATED : EventService.PROFILE_UPDATED;
+ }
+ } catch (IllegalAccessException | NoSuchMethodException | InvocationTargetException e) {
+ logger.warn("Error resolving nested property of object. See debug log level for more information");
+ if (logger.isDebugEnabled()) {
+ logger.error("Error resolving nested property of item: {}", storeInSession ? session : profile, e);
+ }
+ }
+
+ return EventService.NO_CHANGE;
+ }
+
+ private Object getPropertyValue(Action action, Event event, String propertyName, Map<String, Object> properties)
+ throws IllegalAccessException, InvocationTargetException, NoSuchMethodException {
+ String propertyTarget = (String) action.getParameterValues().get("propertyTarget");
+ String rootPropertyName = propertyName.split("\\.")[0];
+ Object propertyValue = 1;
+
+ Object propertyTargetValue = null;
+
+ if (StringUtils.isNotEmpty(propertyTarget)) {
+ propertyTargetValue = PropertyUtils.getNestedProperty(((CustomItem) event.getTarget()).getProperties(), propertyTarget);
+ }
+
+ if (propertyTargetValue != null) {
+ if (propertyTargetValue instanceof Integer) {
+ if (properties.containsKey(rootPropertyName)) {
+ propertyValue = (int) propertyTargetValue + (int) PropertyUtils.getNestedProperty(properties, propertyName);
+ } else {
+ propertyValue = propertyTargetValue;
+ }
+ } else if (propertyTargetValue instanceof Map) {
+ if (properties.containsKey(rootPropertyName)) {
+ Map<String, Object> nestedProperty = (Map<String, Object>) PropertyUtils.getNestedProperty(properties, propertyName);
Review comment:
We should check that PropertyUtils.getNestedProperty(properties, propertyName); is actually not null and it's a map, if not log an error and continue.
##########
File path: itests/src/test/java/org/apache/unomi/itests/IncrementPropertyIT.java
##########
@@ -0,0 +1,411 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+package org.apache.unomi.itests;
+
+import java.util.*;
+
+import javax.inject.Inject;
+
+import org.apache.unomi.api.*;
+import org.apache.unomi.api.actions.Action;
+import org.apache.unomi.api.conditions.Condition;
+import org.apache.unomi.api.rules.Rule;
+import org.apache.unomi.api.services.DefinitionsService;
+import org.apache.unomi.api.services.EventService;
+import org.apache.unomi.api.services.ProfileService;
+import org.apache.unomi.api.services.RulesService;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.ops4j.pax.exam.junit.PaxExam;
+import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy;
+import org.ops4j.pax.exam.spi.reactors.PerSuite;
+import org.ops4j.pax.exam.util.Filter;
+
+import static org.apache.unomi.itests.BasicIT.ITEM_TYPE_PAGE;
+
+@RunWith(PaxExam.class)
+@ExamReactorStrategy(PerSuite.class)
+public class IncrementPropertyIT extends BaseIT {
+ private Profile profile;
+ private Rule rule;
+ private Event event;
+
+ @Inject
+ @Filter(timeout = 600000)
+ protected ProfileService profileService;
+
+ @Inject
+ @Filter(timeout = 600000)
+ protected EventService eventService;
+
+ @Inject
+ @Filter(timeout = 600000)
+ protected RulesService rulesService;
+
+ @Inject
+ @Filter(timeout = 600000)
+ protected DefinitionsService definitionsService;
+
+ @Before
+ public void setup() throws Exception {
+ profile = createProfile();
+ rule = new Rule();
+ }
+
+ @After
+ public void tearDown() {
+ rulesService.removeRule(rule.getItemId());
+ profileService.delete(profile.getItemId(), false);
+ }
+
+ @Test
+ public void testIncrementNotExistingProperty() throws InterruptedException {
+ int eventCode = buildAction("pageView.acme", null, null, null);
+
+ if (eventCode == EventService.PROFILE_UPDATED) {
+ Profile updatedProfile = profileService.save(event.getProfile());
+ refreshPersistence();
+
+ int value = ((Map<String, Integer>) updatedProfile.getProperty("pageView")).get("acme");
+ Assert.assertEquals(1, value, 0.0);
+ } else {
+ throw new IllegalStateException("Profile was not updated");
Review comment:
We should use Assert.fail("the message") instead of Exceptions when something goes wrong in the test. to be fixed on the other test cases too.
##########
File path: plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/IncrementPropertyAction.java
##########
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.unomi.plugins.baseplugin.actions;
+
+import org.apache.commons.beanutils.PropertyUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.unomi.api.*;
+import org.apache.unomi.api.actions.Action;
+import org.apache.unomi.api.actions.ActionExecutor;
+import org.apache.unomi.api.services.EventService;
+import org.apache.unomi.persistence.spi.PropertyHelper;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.InvocationTargetException;
+import java.util.Map;
+
+public class IncrementPropertyAction implements ActionExecutor {
+ private static final Logger logger = LoggerFactory.getLogger(IncrementPropertyAction.class.getName());
+
+ @Override
+ public int execute(final Action action, final Event event) {
+ boolean storeInSession = Boolean.TRUE.equals(action.getParameterValues().get("storeInSession"));
+ if (storeInSession && event.getSession() == null) {
+ return EventService.NO_CHANGE;
+ }
+
+ String propertyName = (String) action.getParameterValues().get("propertyName");
+ Profile profile = event.getProfile();
+ Session session = event.getSession();
+
+ try {
+ Map<String, Object> properties = storeInSession ? session.getProperties() : profile.getProperties();
+ Object propertyValue = getPropertyValue(action, event, propertyName, properties);
+
+ if (PropertyHelper.setProperty(properties, propertyName, propertyValue, "alwaysSet")) {
+ return storeInSession ? EventService.SESSION_UPDATED : EventService.PROFILE_UPDATED;
+ }
+ } catch (IllegalAccessException | NoSuchMethodException | InvocationTargetException e) {
+ logger.warn("Error resolving nested property of object. See debug log level for more information");
+ if (logger.isDebugEnabled()) {
+ logger.error("Error resolving nested property of item: {}", storeInSession ? session : profile, e);
+ }
+ }
+
+ return EventService.NO_CHANGE;
+ }
+
+ private Object getPropertyValue(Action action, Event event, String propertyName, Map<String, Object> properties)
+ throws IllegalAccessException, InvocationTargetException, NoSuchMethodException {
+ String propertyTarget = (String) action.getParameterValues().get("propertyTarget");
+ String rootPropertyName = propertyName.split("\\.")[0];
+ Object propertyValue = 1;
+
+ Object propertyTargetValue = null;
+
+ if (StringUtils.isNotEmpty(propertyTarget)) {
+ propertyTargetValue = PropertyUtils.getNestedProperty(((CustomItem) event.getTarget()).getProperties(), propertyTarget);
+ }
+
+ if (propertyTargetValue != null) {
+ if (propertyTargetValue instanceof Integer) {
+ if (properties.containsKey(rootPropertyName)) {
+ propertyValue = (int) propertyTargetValue + (int) PropertyUtils.getNestedProperty(properties, propertyName);
Review comment:
I would suggest to test that the nestedProperty is not null and of type Integer.
##########
File path: itests/src/test/java/org/apache/unomi/itests/IncrementPropertyIT.java
##########
@@ -0,0 +1,411 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+package org.apache.unomi.itests;
+
+import java.util.*;
+
+import javax.inject.Inject;
+
+import org.apache.unomi.api.*;
+import org.apache.unomi.api.actions.Action;
+import org.apache.unomi.api.conditions.Condition;
+import org.apache.unomi.api.rules.Rule;
+import org.apache.unomi.api.services.DefinitionsService;
+import org.apache.unomi.api.services.EventService;
+import org.apache.unomi.api.services.ProfileService;
+import org.apache.unomi.api.services.RulesService;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.ops4j.pax.exam.junit.PaxExam;
+import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy;
+import org.ops4j.pax.exam.spi.reactors.PerSuite;
+import org.ops4j.pax.exam.util.Filter;
+
+import static org.apache.unomi.itests.BasicIT.ITEM_TYPE_PAGE;
+
+@RunWith(PaxExam.class)
+@ExamReactorStrategy(PerSuite.class)
+public class IncrementPropertyIT extends BaseIT {
+ private Profile profile;
+ private Rule rule;
+ private Event event;
+
+ @Inject
+ @Filter(timeout = 600000)
+ protected ProfileService profileService;
+
+ @Inject
+ @Filter(timeout = 600000)
+ protected EventService eventService;
+
+ @Inject
+ @Filter(timeout = 600000)
+ protected RulesService rulesService;
+
+ @Inject
+ @Filter(timeout = 600000)
+ protected DefinitionsService definitionsService;
+
+ @Before
+ public void setup() throws Exception {
+ profile = createProfile();
+ rule = new Rule();
+ }
+
+ @After
+ public void tearDown() {
+ rulesService.removeRule(rule.getItemId());
+ profileService.delete(profile.getItemId(), false);
+ }
+
+ @Test
+ public void testIncrementNotExistingProperty() throws InterruptedException {
+ int eventCode = buildAction("pageView.acme", null, null, null);
+
+ if (eventCode == EventService.PROFILE_UPDATED) {
+ Profile updatedProfile = profileService.save(event.getProfile());
+ refreshPersistence();
Review comment:
This is not necessary, you dont need to store the profile in test, just check that the profile from the event is correct.
The save is done automatically by the ContextServlet and it is not required to insure the profile contains the correct values.
You can remove this code here and also in the other test cases.
##########
File path: itests/src/test/java/org/apache/unomi/itests/IncrementPropertyIT.java
##########
@@ -0,0 +1,411 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+package org.apache.unomi.itests;
+
+import java.util.*;
+
+import javax.inject.Inject;
+
+import org.apache.unomi.api.*;
+import org.apache.unomi.api.actions.Action;
+import org.apache.unomi.api.conditions.Condition;
+import org.apache.unomi.api.rules.Rule;
+import org.apache.unomi.api.services.DefinitionsService;
+import org.apache.unomi.api.services.EventService;
+import org.apache.unomi.api.services.ProfileService;
+import org.apache.unomi.api.services.RulesService;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.ops4j.pax.exam.junit.PaxExam;
+import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy;
+import org.ops4j.pax.exam.spi.reactors.PerSuite;
+import org.ops4j.pax.exam.util.Filter;
+
+import static org.apache.unomi.itests.BasicIT.ITEM_TYPE_PAGE;
+
+@RunWith(PaxExam.class)
+@ExamReactorStrategy(PerSuite.class)
+public class IncrementPropertyIT extends BaseIT {
+ private Profile profile;
+ private Rule rule;
+ private Event event;
+
+ @Inject
+ @Filter(timeout = 600000)
+ protected ProfileService profileService;
+
+ @Inject
+ @Filter(timeout = 600000)
+ protected EventService eventService;
+
+ @Inject
+ @Filter(timeout = 600000)
+ protected RulesService rulesService;
+
+ @Inject
+ @Filter(timeout = 600000)
+ protected DefinitionsService definitionsService;
+
+ @Before
+ public void setup() throws Exception {
+ profile = createProfile();
+ rule = new Rule();
+ }
+
+ @After
+ public void tearDown() {
+ rulesService.removeRule(rule.getItemId());
+ profileService.delete(profile.getItemId(), false);
+ }
+
+ @Test
+ public void testIncrementNotExistingProperty() throws InterruptedException {
+ int eventCode = buildAction("pageView.acme", null, null, null);
+
+ if (eventCode == EventService.PROFILE_UPDATED) {
+ Profile updatedProfile = profileService.save(event.getProfile());
+ refreshPersistence();
+
+ int value = ((Map<String, Integer>) updatedProfile.getProperty("pageView")).get("acme");
+ Assert.assertEquals(1, value, 0.0);
+ } else {
+ throw new IllegalStateException("Profile was not updated");
+ }
+ }
+
+ @Test
+ public void testIncrementExistingProperty() throws InterruptedException {
+ Map<String, Object> properties = new HashMap<>();
+ Map<String, Integer> propertyValue = new HashMap<>();
+ propertyValue.put("acme", 49);
+ properties.put("pageView", propertyValue);
+
+ int eventCode = buildAction("pageView.acme", null, properties, null);
+
+ if (eventCode == EventService.PROFILE_UPDATED) {
+ Profile updatedProfile = profileService.save(event.getProfile());
+
+ refreshPersistence();
+
+ int value = ((Map<String, Integer>) updatedProfile.getProperty("pageView")).get("acme");
+ Assert.assertEquals(50, value, 0.0);
+ } else {
+ throw new IllegalStateException("Profile was not updated");
+ }
+ }
+
+ @Test
+ public void testIncrementExistingPropertyWithExistingEventProperty() throws InterruptedException {
+ Map<String, Object> properties = new HashMap<>();
+ Map<String, Integer> propertyValue = new HashMap<>();
+ propertyValue.put("acme", 49);
+ properties.put("pageView", propertyValue);
+
+ Map<String, Object> targetProperties = new HashMap<>();
+ propertyValue = new HashMap<>();
+ propertyValue.put("nasa", 19);
+ targetProperties.put("project", propertyValue);
+
+ int eventCode = buildAction("pageView.acme", "project.nasa", properties, targetProperties);
+
+ if (eventCode == EventService.PROFILE_UPDATED) {
+ Profile updatedProfile = profileService.save(event.getProfile());
+
+ refreshPersistence();
+
+ int value = ((Map<String, Integer>) updatedProfile.getProperty("pageView")).get("acme");
+ Assert.assertEquals(68, value, 0.0);
+ } else {
+ throw new IllegalStateException("Profile was not updated");
+ }
+ }
+
+ @Test
+ public void testIncrementNotExistingObjectPropertyWithExistingEventObjectProperty() throws InterruptedException {
+ Map<String, Object> targetProperties = new HashMap<>();
+ Map<String, Integer> propertyValue = new HashMap<>();
+ propertyValue.put("acme", 49);
+ propertyValue.put("health", 18);
+ propertyValue.put("sport", 99);
+ targetProperties.put("pageView", propertyValue);
+
+ int eventCode = buildAction("pageView", "pageView", null, targetProperties);
+
+ if (eventCode == EventService.PROFILE_UPDATED) {
+ Profile updatedProfile = profileService.save(event.getProfile());
+
+ refreshPersistence();
+
+ Map<String, Integer> property = ((Map<String, Integer>) updatedProfile.getProperty("pageView"));
+ Assert.assertEquals(49, property.get("acme"), 0.0);
+ Assert.assertEquals(18, property.get("health"), 0.0);
+ Assert.assertEquals(99, property.get("sport"), 0.0);
+ } else {
+ throw new IllegalStateException("Profile was not updated");
+ }
+ }
+
+ @Test
+ public void testIncrementExistingObjectProperty() throws InterruptedException {
+ Map<String, Object> properties = new HashMap<>();
+ Map<String, Integer> propertyValue = new HashMap<>();
+ propertyValue.put("acme", 49);
+ propertyValue.put("nasa", 5);
+ properties.put("pageView", propertyValue);
+
+ int eventCode = buildAction("pageView", null, properties, null);
+
+ if (eventCode == EventService.PROFILE_UPDATED) {
+ Profile updatedProfile = profileService.save(event.getProfile());
+
+ refreshPersistence();
+
+ Map<String, Integer> property = ((Map<String, Integer>) updatedProfile.getProperty("pageView"));
+ Assert.assertEquals(50, property.get("acme"), 0.0);
+ Assert.assertEquals(6, property.get("nasa"), 0.0);
+ } else {
+ throw new IllegalStateException("Profile was not updated");
+ }
+ }
+
+ @Test
+ public void testIncrementExistingObjectPropertyWithExistingEventObjectProperty() throws InterruptedException {
+ Map<String, Object> properties = new HashMap<>();
+ Map<String, Integer> propertyValue = new HashMap<>();
+ propertyValue.put("acme", 49);
+ properties.put("pageView", propertyValue);
+
+ Map<String, Object> targetProperties = new HashMap<>();
+ Map<String, Integer> propertyValue1 = new HashMap<>();
+ propertyValue1.put("acme", 31);
+ propertyValue1.put("health", 88);
+ propertyValue1.put("sport", 9);
+ targetProperties.put("pageView", propertyValue1);
+
+ int eventCode = buildAction("pageView", "pageView", properties, targetProperties);
+
+ if (eventCode == EventService.PROFILE_UPDATED) {
+ Profile updatedProfile = profileService.save(event.getProfile());
+
+ refreshPersistence();
+
+ Map<String, Integer> property = ((Map<String, Integer>) updatedProfile.getProperty("pageView"));
+ Assert.assertEquals(80, property.get("acme"), 0.0);
+ Assert.assertEquals(88, property.get("health"), 0.0);
+ Assert.assertEquals(9, property.get("sport"), 0.0);
+ } else {
+ throw new IllegalStateException("Profile was not updated");
+ }
+ }
+
+ @Test
+ public void testIncrementExistingPropertyNested() throws InterruptedException {
+ Map<String, Object> properties = new HashMap<>();
+ Map<String, Object> properties1 = new HashMap<>();
+ Map<String, Object> properties2 = new HashMap<>();
+ Map<String, Integer> propertyValue = new HashMap<>();
+ propertyValue.put("city", 13);
+ properties2.put("state", propertyValue);
+ properties1.put("country", properties2);
+ properties.put("continent", properties1);
+
+ int eventCode = buildAction("continent.country.state.city", null, properties, null);
+
+ if (eventCode == EventService.PROFILE_UPDATED) {
+ Profile updatedProfile = profileService.save(event.getProfile());
+
+ refreshPersistence();
+
+ Map<String, Integer> property = (Map<String, Integer>) ((Map<String, Object>) ((Map<String, Object>) updatedProfile.getProperty("continent")).get("country")).get("state");
+ Assert.assertEquals(14, property.get("city"), 0.0);
+ } else {
+ throw new IllegalStateException("Profile was not updated");
+ }
+ }
+
+ @Test
+ public void testIncrementNotExistingPropertyNested() throws InterruptedException {
+ int eventCode = buildAction("continent.country.state.city", null, null, null);
+
+ if (eventCode == EventService.PROFILE_UPDATED) {
+ Profile updatedProfile = profileService.save(event.getProfile());
+
+ refreshPersistence();
+
+ Map<String, Integer> property = (Map<String, Integer>) ((Map<String, Object>) ((Map<String, Object>) updatedProfile.getProperty("continent")).get("country")).get("state");
+ Assert.assertEquals(1, property.get("city"), 0.0);
+ } else {
+ throw new IllegalStateException("Profile was not updated");
+ }
+ }
+
+ @Test
+ public void testIncrementExistingPropertyNestedWithExistingEventProperty() throws InterruptedException {
+ Map<String, Object> properties = new HashMap<>();
+ Map<String, Object> properties1 = new HashMap<>();
+ Map<String, Object> properties2 = new HashMap<>();
+ Map<String, Integer> propertyValue = new HashMap<>();
+ propertyValue.put("city", 13);
+ properties2.put("state", propertyValue);
+ properties1.put("country", properties2);
+ properties.put("continent", properties1);
+
+ Map<String, Object> targetProperties = new HashMap<>();
+ Map<String, Object> properties3 = new HashMap<>();
+ Map<String, Object> propertyValue1 = new HashMap<>();
+ propertyValue1.put("zone", 107);
+ properties3.put("mars", propertyValue1);
+ targetProperties.put("planet", properties3);
+
+ int eventCode = buildAction("continent.country.state.city", "planet.mars.zone", properties, targetProperties);
+
+ if (eventCode == EventService.PROFILE_UPDATED) {
+ Profile updatedProfile = profileService.save(event.getProfile());
+
+ refreshPersistence();
+
+ Map<String, Integer> property = (Map<String, Integer>) ((Map<String, Object>) ((Map<String, Object>) updatedProfile.getProperty("continent")).get("country")).get("state");
+ Assert.assertEquals(120, property.get("city"), 0.0);
+ } else {
+ throw new IllegalStateException("Profile was not updated");
+ }
+ }
+
+ @Test
+ public void testIncrementObjectPropertyContainsStringValue() throws InterruptedException {
+ Map<String, Object> properties = new HashMap<>();
+ Map<String, Object> propertyValue = new HashMap<>();
+ propertyValue.put("books", 59);
+ propertyValue.put("chapters", 1001);
+ propertyValue.put("featured", "The forty rules");
+ properties.put("library", propertyValue);
+
+ int eventCode = buildAction("library", null, properties, null);
+
+ if (eventCode == EventService.PROFILE_UPDATED) {
+ Profile updatedProfile = profileService.save(event.getProfile());
+
+ refreshPersistence();
+
+ Map<String, Object> property = ((Map<String, Object>) updatedProfile.getProperty("library"));
+ Assert.assertEquals(60, (int) property.get("books"), 0.0);
+ Assert.assertEquals(1002, (int) property.get("chapters"), 0.0);
+ Assert.assertEquals("The forty rules", property.get("featured"));
+ } else {
+ throw new IllegalStateException("Profile was not updated");
+ }
+ }
+
+ @Test
+ public void testIncrementObjectPropertyContainsStringValueWithExistingEventProperty() throws InterruptedException {
+ Map<String, Object> properties = new HashMap<>();
+ Map<String, Object> propertyValue = new HashMap<>();
+ propertyValue.put("books", 59);
+ propertyValue.put("chapters", 1001);
+ propertyValue.put("featured", "The forty rules");
+ properties.put("library", propertyValue);
+
+ Map<String, Object> targetProperties = new HashMap<>();
+ Map<String, Object> properties1 = new HashMap<>();
+ Map<String, Object> propertyValue1 = new HashMap<>();
+ propertyValue1.put("books", 222);
+ propertyValue1.put("chapters", 2048);
+ propertyValue1.put("featured", "Bible");
+ properties1.put("library", propertyValue1);
+ targetProperties.put("main", properties1);
+
+ int eventCode = buildAction("library", "main.library", properties, targetProperties);
+
+ if (eventCode == EventService.PROFILE_UPDATED) {
+ Profile updatedProfile = profileService.save(event.getProfile());
+
+ refreshPersistence();
+
+ Map<String, Object> property = ((Map<String, Object>) updatedProfile.getProperty("library"));
+ Assert.assertEquals(281, (int) property.get("books"), 0.0);
+ Assert.assertEquals(3049, (int) property.get("chapters"), 0.0);
+ Assert.assertEquals("The forty rules", property.get("featured"));
+ } else {
+ throw new IllegalStateException("Profile was not updated");
+ }
+ }
+
+ private void createRule(Action incrementPropertyAction) throws InterruptedException {
+ Condition condition = createCondition();
+ Metadata metadata = createMetadata();
+
+ List<Action> actions = new ArrayList<>();
+ actions.add(incrementPropertyAction);
+
+ rule.setCondition(condition);
+ rule.setActions(actions);
+ rule.setMetadata(metadata);
+ rulesService.setRule(rule);
+ refreshPersistence();
+ }
+
+ private int buildAction(String propertyName, String propertyTargetName, Map<String, Object> properties, Map<String, Object> targetProperties) throws InterruptedException {
Review comment:
Would rename "buildActionAndSendEvent"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [unomi] jkevan edited a comment on pull request #265: UNOMI-446 improve increment action and make it generic
Posted by GitBox <gi...@apache.org>.
jkevan edited a comment on pull request #265:
URL: https://github.com/apache/unomi/pull/265#issuecomment-804209534
### Small update on what we discussed about in the call:
- Do not touch the unomi IncrementInterestAction. This action have his own logic and cannot be generify with property incrementation, also the interest incrementation from JExperience is not using this action, it's actually using a script and the setPropertyAction.
- Create a new Action: IncrementPropertyAction:
- propertyName: to be able to increment or create a single prop (properties.pageViewCount)
- rootPropertyName: to be able to increment all props under this one: (ex: profile.properties.interest)
- incrementWithValues: to be able to increment properties with the values coming from the event. (ex: event.properties.interest)
- storeInSession: to be able to target the profile or the session.
### Real exemple coming from JEXperience:
**the increment page count:**
```
"actions": [
{
"parameterValues": {
"setPropertyName": "properties.pageViewCount",
"setPropertyValue": "script::r = profile.properties['pageViewCount']; if (r == null) { profile.properties['pageViewCount'] = []; profile.properties.pageViewCount = [event.scope : 1] } else { if (r[event.scope] != null) { r[event.scope] = r[event.scope] + 1 } else { r[event.scope] = 1 }} r",
"storeInSession": false
},
"type": "setPropertyAction"
}
]
```
We should be able to do it with this kind of conf:
**type: incrementPropertyAction**
**paramerterValues:**
- **propertyName: properties.pageViewCount**
- **storeInSession: false**
**The increment interest from jexperience:**
```
"actions": [
{
"parameterValues": {
"setPropertyName": "properties.interests",
"setPropertyValue": "script::r = profile.properties['interests']; foreach(interest : event.target.properties['interests'].entrySet()) { if (r == null) { r = [interest.key: interest.value] } else if (r[interest.key] != null) { r[interest.key] = r[interest.key] + interest.value } else { r[interest.key] = interest.value } } r",
"storeInSession": false
},
"type": "setPropertyAction"
}
]
```
We should be able to replace it, with:
**type:incrementPropertyAction**
**paramerterValues:**
- **rootPropertyName: properties.interest**
- **incrementWithEventValues: target.properties.interest**
- **storeInSession: false**
This way we will be able to replace the script usage we have in the rules in JExperience to use a generic increment action.
The Existing IncrementInterestAction is missleading, and confusing because there is also interest incrementation in jexperience. But the both are completely differents.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [unomi] sergehuber commented on a change in pull request #265: UNOMI-446 improve increment action and make it generic
Posted by GitBox <gi...@apache.org>.
sergehuber commented on a change in pull request #265:
URL: https://github.com/apache/unomi/pull/265#discussion_r598800936
##########
File path: services/src/main/java/org/apache/unomi/services/actions/ActionExecutorDispatcher.java
##########
@@ -146,6 +156,19 @@ public Action getContextualAction(Action action, Event event) {
return values;
}
+ private Object extractValue(String s, Event event) throws IllegalAccessException, NoSuchMethodException, InvocationTargetException {
+ Object value = null;
+
+ String valueType = StringUtils.substringBefore(s, VALUE_NAME_SEPARATOR);
+ String valueAsString = StringUtils.substringAfter(s, VALUE_NAME_SEPARATOR);
+ ValueExtractor extractor = valueExtractors.get(valueType);
+ if (extractor != null) {
+ value = extractor.extract(valueAsString, event);
+ }
+
+ return value;
Review comment:
We should check the value to make sure it doesn't have any PLACEHOLDER_PREFIX in which case we might need to encode it somehow, otherwise this could lead to a security vulnerability as attackers could inject expressions that could ready any data.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [unomi] jkevan commented on a change in pull request #265: UNOMI-446 improve increment action and make it generic
Posted by GitBox <gi...@apache.org>.
jkevan commented on a change in pull request #265:
URL: https://github.com/apache/unomi/pull/265#discussion_r600770756
##########
File path: plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/IncrementPropertyAction.java
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.unomi.plugins.baseplugin.actions;
+
+import org.apache.commons.beanutils.PropertyUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.unomi.api.CustomItem;
+import org.apache.unomi.api.Event;
+import org.apache.unomi.api.Profile;
+import org.apache.unomi.api.actions.Action;
+import org.apache.unomi.api.actions.ActionExecutor;
+import org.apache.unomi.api.services.EventService;
+import org.apache.unomi.persistence.spi.PropertyHelper;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.InvocationTargetException;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Map;
+
+public class IncrementPropertyAction implements ActionExecutor {
+ private static final Logger logger = LoggerFactory.getLogger(IncrementPropertyAction.class.getName());
+ private EventService eventService;
+
+ @Override
+ public int execute(final Action action, final Event event) {
+ boolean storeInSession = Boolean.TRUE.equals(action.getParameterValues().get("storeInSession"));
+ if (storeInSession && event.getSession() == null) {
+ return EventService.NO_CHANGE;
+ }
+
+ Profile profile = event.getProfile();
+ String propertyName = (String) action.getParameterValues().get("propertyName");
+ String propertyTarget = (String) action.getParameterValues().get("propertyTarget");
+ String rootPropertyName = propertyName.split("\\.")[0];
+ Object value = null;
+ Object propertyValue = 1;
+
+ try {
+ if (StringUtils.isNotEmpty(propertyTarget)) {
+ value = PropertyUtils.getNestedProperty(((CustomItem) event.getTarget()).getProperties(), propertyTarget);
+ }
+
+ if (value != null) {
+ if (value instanceof Integer) {
+ if (profile.getProperty(rootPropertyName) != null) {
+ propertyValue = (int) value + (int) PropertyUtils.getNestedProperty(profile.getProperties(), propertyName);
+ } else {
+ propertyValue = value;
+ }
+ } else if (value instanceof Map) {
+ if (profile.getProperty(rootPropertyName) != null) {
+ Map<String, Integer> p = (Map<String, Integer>) PropertyUtils.getNestedProperty(profile.getProperties(), propertyName);
+ ((Map<String, Integer>) value).forEach((k, v) -> p.put(k, p.containsKey(k) ? p.get(k) + v : v));
+
+ propertyValue = p;
+ } else {
+ propertyValue = value;
+ }
+ }
+ } else {
+ if (profile.getProperty(rootPropertyName) != null) {
+ Object p = PropertyUtils.getNestedProperty(profile.getProperties(), propertyName);
+ if (p instanceof Integer) {
+ propertyValue = (int) p + 1;
+ } else if (p instanceof Map) {
+ ((Map<String, Integer>) p).forEach((k, v) -> ((Map<String, Integer>) p).merge(k, 1, Integer::sum));
+ propertyValue = p;
+ }
+ }
+ }
+
+ PropertyHelper.setProperty(profile.getProperties(), propertyName, propertyValue, "alwaysSet");
+
+ Object rootPropertyValue = PropertyUtils.getNestedProperty(profile.getProperties(), rootPropertyName);
+ if (storeInSession) {
+ if (PropertyHelper.setProperty(event.getSession(), rootPropertyName, rootPropertyValue, "alwaysSet")) {
+ return EventService.SESSION_UPDATED;
+ }
+ } else {
+ Event updatePropertiesEvent = new Event("updateProperties", event.getSession(), profile, event.getSourceId(), null, event.getTarget(), new Date());
+ Map<String, Object> propertyToUpdate = new HashMap<>();
+ propertyToUpdate.put("properties." + rootPropertyName, rootPropertyValue);
+ updatePropertiesEvent.setProperty(UpdatePropertiesAction.PROPS_TO_UPDATE, propertyToUpdate);
+
+ return eventService.send(updatePropertiesEvent);
Review comment:
Why do we send an event for updating the property here ?
it should be the work of the current action to do it, no additional event should be send to perform the update.
##########
File path: plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/IncrementPropertyAction.java
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.unomi.plugins.baseplugin.actions;
+
+import org.apache.commons.beanutils.PropertyUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.unomi.api.CustomItem;
+import org.apache.unomi.api.Event;
+import org.apache.unomi.api.Profile;
+import org.apache.unomi.api.actions.Action;
+import org.apache.unomi.api.actions.ActionExecutor;
+import org.apache.unomi.api.services.EventService;
+import org.apache.unomi.persistence.spi.PropertyHelper;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.InvocationTargetException;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Map;
+
+public class IncrementPropertyAction implements ActionExecutor {
+ private static final Logger logger = LoggerFactory.getLogger(IncrementPropertyAction.class.getName());
+ private EventService eventService;
+
+ @Override
+ public int execute(final Action action, final Event event) {
+ boolean storeInSession = Boolean.TRUE.equals(action.getParameterValues().get("storeInSession"));
+ if (storeInSession && event.getSession() == null) {
+ return EventService.NO_CHANGE;
+ }
+
+ Profile profile = event.getProfile();
+ String propertyName = (String) action.getParameterValues().get("propertyName");
+ String propertyTarget = (String) action.getParameterValues().get("propertyTarget");
+ String rootPropertyName = propertyName.split("\\.")[0];
+ Object value = null;
+ Object propertyValue = 1;
+
+ try {
+ if (StringUtils.isNotEmpty(propertyTarget)) {
+ value = PropertyUtils.getNestedProperty(((CustomItem) event.getTarget()).getProperties(), propertyTarget);
+ }
+
+ if (value != null) {
+ if (value instanceof Integer) {
+ if (profile.getProperty(rootPropertyName) != null) {
+ propertyValue = (int) value + (int) PropertyUtils.getNestedProperty(profile.getProperties(), propertyName);
+ } else {
+ propertyValue = value;
+ }
+ } else if (value instanceof Map) {
+ if (profile.getProperty(rootPropertyName) != null) {
+ Map<String, Integer> p = (Map<String, Integer>) PropertyUtils.getNestedProperty(profile.getProperties(), propertyName);
+ ((Map<String, Integer>) value).forEach((k, v) -> p.put(k, p.containsKey(k) ? p.get(k) + v : v));
+
+ propertyValue = p;
+ } else {
+ propertyValue = value;
+ }
+ }
+ } else {
+ if (profile.getProperty(rootPropertyName) != null) {
+ Object p = PropertyUtils.getNestedProperty(profile.getProperties(), propertyName);
+ if (p instanceof Integer) {
Review comment:
Should we create the prop if it doesnt exist (p == null)?
input:
propName = toto
profile.toto = null
should the output be :
profile.toto = 1 ?
##########
File path: plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/IncrementPropertyAction.java
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.unomi.plugins.baseplugin.actions;
+
+import org.apache.commons.beanutils.PropertyUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.unomi.api.CustomItem;
+import org.apache.unomi.api.Event;
+import org.apache.unomi.api.Profile;
+import org.apache.unomi.api.actions.Action;
+import org.apache.unomi.api.actions.ActionExecutor;
+import org.apache.unomi.api.services.EventService;
+import org.apache.unomi.persistence.spi.PropertyHelper;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.InvocationTargetException;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Map;
+
+public class IncrementPropertyAction implements ActionExecutor {
+ private static final Logger logger = LoggerFactory.getLogger(IncrementPropertyAction.class.getName());
+ private EventService eventService;
+
+ @Override
+ public int execute(final Action action, final Event event) {
+ boolean storeInSession = Boolean.TRUE.equals(action.getParameterValues().get("storeInSession"));
+ if (storeInSession && event.getSession() == null) {
+ return EventService.NO_CHANGE;
+ }
+
+ Profile profile = event.getProfile();
+ String propertyName = (String) action.getParameterValues().get("propertyName");
+ String propertyTarget = (String) action.getParameterValues().get("propertyTarget");
+ String rootPropertyName = propertyName.split("\\.")[0];
+ Object value = null;
+ Object propertyValue = 1;
+
+ try {
+ if (StringUtils.isNotEmpty(propertyTarget)) {
+ value = PropertyUtils.getNestedProperty(((CustomItem) event.getTarget()).getProperties(), propertyTarget);
+ }
+
+ if (value != null) {
+ if (value instanceof Integer) {
+ if (profile.getProperty(rootPropertyName) != null) {
+ propertyValue = (int) value + (int) PropertyUtils.getNestedProperty(profile.getProperties(), propertyName);
+ } else {
+ propertyValue = value;
+ }
+ } else if (value instanceof Map) {
+ if (profile.getProperty(rootPropertyName) != null) {
+ Map<String, Integer> p = (Map<String, Integer>) PropertyUtils.getNestedProperty(profile.getProperties(), propertyName);
+ ((Map<String, Integer>) value).forEach((k, v) -> p.put(k, p.containsKey(k) ? p.get(k) + v : v));
+
+ propertyValue = p;
+ } else {
+ propertyValue = value;
+ }
+ }
+ } else {
+ if (profile.getProperty(rootPropertyName) != null) {
+ Object p = PropertyUtils.getNestedProperty(profile.getProperties(), propertyName);
+ if (p instanceof Integer) {
+ propertyValue = (int) p + 1;
+ } else if (p instanceof Map) {
+ ((Map<String, Integer>) p).forEach((k, v) -> ((Map<String, Integer>) p).merge(k, 1, Integer::sum));
+ propertyValue = p;
+ }
+ }
+ }
+
+ PropertyHelper.setProperty(profile.getProperties(), propertyName, propertyValue, "alwaysSet");
+
+ Object rootPropertyValue = PropertyUtils.getNestedProperty(profile.getProperties(), rootPropertyName);
+ if (storeInSession) {
+ if (PropertyHelper.setProperty(event.getSession(), rootPropertyName, rootPropertyValue, "alwaysSet")) {
+ return EventService.SESSION_UPDATED;
+ }
+ } else {
+ Event updatePropertiesEvent = new Event("updateProperties", event.getSession(), profile, event.getSourceId(), null, event.getTarget(), new Date());
+ Map<String, Object> propertyToUpdate = new HashMap<>();
+ propertyToUpdate.put("properties." + rootPropertyName, rootPropertyValue);
+ updatePropertiesEvent.setProperty(UpdatePropertiesAction.PROPS_TO_UPDATE, propertyToUpdate);
+
+ return eventService.send(updatePropertiesEvent);
+ }
+ } catch (IllegalAccessException | NoSuchMethodException | InvocationTargetException e) {
+ logger.error("Error resolving nested property of profile: {}", profile, e);
+ }
+
+ return EventService.NO_CHANGE;
Review comment:
We should return changes in case we did some incrementations on the profile
##########
File path: plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/IncrementPropertyAction.java
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.unomi.plugins.baseplugin.actions;
+
+import org.apache.commons.beanutils.PropertyUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.unomi.api.CustomItem;
+import org.apache.unomi.api.Event;
+import org.apache.unomi.api.Profile;
+import org.apache.unomi.api.actions.Action;
+import org.apache.unomi.api.actions.ActionExecutor;
+import org.apache.unomi.api.services.EventService;
+import org.apache.unomi.persistence.spi.PropertyHelper;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.InvocationTargetException;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Map;
+
+public class IncrementPropertyAction implements ActionExecutor {
+ private static final Logger logger = LoggerFactory.getLogger(IncrementPropertyAction.class.getName());
+ private EventService eventService;
+
+ @Override
+ public int execute(final Action action, final Event event) {
+ boolean storeInSession = Boolean.TRUE.equals(action.getParameterValues().get("storeInSession"));
+ if (storeInSession && event.getSession() == null) {
+ return EventService.NO_CHANGE;
+ }
+
+ Profile profile = event.getProfile();
+ String propertyName = (String) action.getParameterValues().get("propertyName");
+ String propertyTarget = (String) action.getParameterValues().get("propertyTarget");
+ String rootPropertyName = propertyName.split("\\.")[0];
+ Object value = null;
+ Object propertyValue = 1;
+
+ try {
+ if (StringUtils.isNotEmpty(propertyTarget)) {
+ value = PropertyUtils.getNestedProperty(((CustomItem) event.getTarget()).getProperties(), propertyTarget);
+ }
+
+ if (value != null) {
+ if (value instanceof Integer) {
+ if (profile.getProperty(rootPropertyName) != null) {
+ propertyValue = (int) value + (int) PropertyUtils.getNestedProperty(profile.getProperties(), propertyName);
+ } else {
+ propertyValue = value;
+ }
+ } else if (value instanceof Map) {
+ if (profile.getProperty(rootPropertyName) != null) {
+ Map<String, Integer> p = (Map<String, Integer>) PropertyUtils.getNestedProperty(profile.getProperties(), propertyName);
+ ((Map<String, Integer>) value).forEach((k, v) -> p.put(k, p.containsKey(k) ? p.get(k) + v : v));
Review comment:
Could be dangerous in case there is one prop that is not an integer, may be it could be more safe to iterate on all the nested props and check if they are numbers. (same for the profile)
##########
File path: plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/IncrementPropertyAction.java
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.unomi.plugins.baseplugin.actions;
+
+import org.apache.commons.beanutils.PropertyUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.unomi.api.CustomItem;
+import org.apache.unomi.api.Event;
+import org.apache.unomi.api.Profile;
+import org.apache.unomi.api.actions.Action;
+import org.apache.unomi.api.actions.ActionExecutor;
+import org.apache.unomi.api.services.EventService;
+import org.apache.unomi.persistence.spi.PropertyHelper;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.InvocationTargetException;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Map;
+
+public class IncrementPropertyAction implements ActionExecutor {
+ private static final Logger logger = LoggerFactory.getLogger(IncrementPropertyAction.class.getName());
+ private EventService eventService;
+
+ @Override
+ public int execute(final Action action, final Event event) {
+ boolean storeInSession = Boolean.TRUE.equals(action.getParameterValues().get("storeInSession"));
+ if (storeInSession && event.getSession() == null) {
+ return EventService.NO_CHANGE;
+ }
+
+ Profile profile = event.getProfile();
+ String propertyName = (String) action.getParameterValues().get("propertyName");
+ String propertyTarget = (String) action.getParameterValues().get("propertyTarget");
+ String rootPropertyName = propertyName.split("\\.")[0];
+ Object value = null;
+ Object propertyValue = 1;
+
+ try {
+ if (StringUtils.isNotEmpty(propertyTarget)) {
+ value = PropertyUtils.getNestedProperty(((CustomItem) event.getTarget()).getProperties(), propertyTarget);
+ }
+
+ if (value != null) {
+ if (value instanceof Integer) {
+ if (profile.getProperty(rootPropertyName) != null) {
+ propertyValue = (int) value + (int) PropertyUtils.getNestedProperty(profile.getProperties(), propertyName);
+ } else {
+ propertyValue = value;
+ }
+ } else if (value instanceof Map) {
+ if (profile.getProperty(rootPropertyName) != null) {
+ Map<String, Integer> p = (Map<String, Integer>) PropertyUtils.getNestedProperty(profile.getProperties(), propertyName);
+ ((Map<String, Integer>) value).forEach((k, v) -> p.put(k, p.containsKey(k) ? p.get(k) + v : v));
+
+ propertyValue = p;
+ } else {
+ propertyValue = value;
+ }
+ }
+ } else {
+ if (profile.getProperty(rootPropertyName) != null) {
+ Object p = PropertyUtils.getNestedProperty(profile.getProperties(), propertyName);
+ if (p instanceof Integer) {
+ propertyValue = (int) p + 1;
+ } else if (p instanceof Map) {
+ ((Map<String, Integer>) p).forEach((k, v) -> ((Map<String, Integer>) p).merge(k, 1, Integer::sum));
+ propertyValue = p;
+ }
+ }
+ }
+
+ PropertyHelper.setProperty(profile.getProperties(), propertyName, propertyValue, "alwaysSet");
+
+ Object rootPropertyValue = PropertyUtils.getNestedProperty(profile.getProperties(), rootPropertyName);
+ if (storeInSession) {
+ if (PropertyHelper.setProperty(event.getSession(), rootPropertyName, rootPropertyValue, "alwaysSet")) {
+ return EventService.SESSION_UPDATED;
+ }
Review comment:
I dont understand this code, if the incrementation have to be done on a session property, it should be done on the session directly in the previous code not on the profile.
The increment property action, should be able to increment a property of the profile, but also increment a property of the session.
We dont want to store the profile props in then session.
We want to be able to increment a session property.
##########
File path: plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/IncrementPropertyAction.java
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.unomi.plugins.baseplugin.actions;
+
+import org.apache.commons.beanutils.PropertyUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.unomi.api.CustomItem;
+import org.apache.unomi.api.Event;
+import org.apache.unomi.api.Profile;
+import org.apache.unomi.api.actions.Action;
+import org.apache.unomi.api.actions.ActionExecutor;
+import org.apache.unomi.api.services.EventService;
+import org.apache.unomi.persistence.spi.PropertyHelper;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.InvocationTargetException;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Map;
+
+public class IncrementPropertyAction implements ActionExecutor {
+ private static final Logger logger = LoggerFactory.getLogger(IncrementPropertyAction.class.getName());
+ private EventService eventService;
+
+ @Override
+ public int execute(final Action action, final Event event) {
+ boolean storeInSession = Boolean.TRUE.equals(action.getParameterValues().get("storeInSession"));
+ if (storeInSession && event.getSession() == null) {
+ return EventService.NO_CHANGE;
+ }
+
+ Profile profile = event.getProfile();
+ String propertyName = (String) action.getParameterValues().get("propertyName");
+ String propertyTarget = (String) action.getParameterValues().get("propertyTarget");
+ String rootPropertyName = propertyName.split("\\.")[0];
+ Object value = null;
Review comment:
Provide better name for the variable for better understanding the code could be great, this is the target value from the event
##########
File path: plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/IncrementPropertyAction.java
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.unomi.plugins.baseplugin.actions;
+
+import org.apache.commons.beanutils.PropertyUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.unomi.api.CustomItem;
+import org.apache.unomi.api.Event;
+import org.apache.unomi.api.Profile;
+import org.apache.unomi.api.actions.Action;
+import org.apache.unomi.api.actions.ActionExecutor;
+import org.apache.unomi.api.services.EventService;
+import org.apache.unomi.persistence.spi.PropertyHelper;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.InvocationTargetException;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Map;
+
+public class IncrementPropertyAction implements ActionExecutor {
+ private static final Logger logger = LoggerFactory.getLogger(IncrementPropertyAction.class.getName());
+ private EventService eventService;
+
+ @Override
+ public int execute(final Action action, final Event event) {
+ boolean storeInSession = Boolean.TRUE.equals(action.getParameterValues().get("storeInSession"));
+ if (storeInSession && event.getSession() == null) {
+ return EventService.NO_CHANGE;
+ }
+
+ Profile profile = event.getProfile();
+ String propertyName = (String) action.getParameterValues().get("propertyName");
+ String propertyTarget = (String) action.getParameterValues().get("propertyTarget");
+ String rootPropertyName = propertyName.split("\\.")[0];
+ Object value = null;
+ Object propertyValue = 1;
+
+ try {
+ if (StringUtils.isNotEmpty(propertyTarget)) {
+ value = PropertyUtils.getNestedProperty(((CustomItem) event.getTarget()).getProperties(), propertyTarget);
+ }
+
+ if (value != null) {
+ if (value instanceof Integer) {
+ if (profile.getProperty(rootPropertyName) != null) {
+ propertyValue = (int) value + (int) PropertyUtils.getNestedProperty(profile.getProperties(), propertyName);
+ } else {
+ propertyValue = value;
+ }
+ } else if (value instanceof Map) {
+ if (profile.getProperty(rootPropertyName) != null) {
+ Map<String, Integer> p = (Map<String, Integer>) PropertyUtils.getNestedProperty(profile.getProperties(), propertyName);
+ ((Map<String, Integer>) value).forEach((k, v) -> p.put(k, p.containsKey(k) ? p.get(k) + v : v));
+
+ propertyValue = p;
+ } else {
+ propertyValue = value;
+ }
+ }
+ } else {
+ if (profile.getProperty(rootPropertyName) != null) {
+ Object p = PropertyUtils.getNestedProperty(profile.getProperties(), propertyName);
+ if (p instanceof Integer) {
+ propertyValue = (int) p + 1;
+ } else if (p instanceof Map) {
+ ((Map<String, Integer>) p).forEach((k, v) -> ((Map<String, Integer>) p).merge(k, 1, Integer::sum));
Review comment:
we should check that the actual nested values are integers before doing the sum
##########
File path: plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/IncrementPropertyAction.java
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.unomi.plugins.baseplugin.actions;
+
+import org.apache.commons.beanutils.PropertyUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.unomi.api.CustomItem;
+import org.apache.unomi.api.Event;
+import org.apache.unomi.api.Profile;
+import org.apache.unomi.api.actions.Action;
+import org.apache.unomi.api.actions.ActionExecutor;
+import org.apache.unomi.api.services.EventService;
+import org.apache.unomi.persistence.spi.PropertyHelper;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.InvocationTargetException;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Map;
+
+public class IncrementPropertyAction implements ActionExecutor {
+ private static final Logger logger = LoggerFactory.getLogger(IncrementPropertyAction.class.getName());
+ private EventService eventService;
+
+ @Override
+ public int execute(final Action action, final Event event) {
+ boolean storeInSession = Boolean.TRUE.equals(action.getParameterValues().get("storeInSession"));
+ if (storeInSession && event.getSession() == null) {
+ return EventService.NO_CHANGE;
+ }
+
+ Profile profile = event.getProfile();
+ String propertyName = (String) action.getParameterValues().get("propertyName");
+ String propertyTarget = (String) action.getParameterValues().get("propertyTarget");
+ String rootPropertyName = propertyName.split("\\.")[0];
+ Object value = null;
+ Object propertyValue = 1;
+
+ try {
+ if (StringUtils.isNotEmpty(propertyTarget)) {
+ value = PropertyUtils.getNestedProperty(((CustomItem) event.getTarget()).getProperties(), propertyTarget);
+ }
+
+ if (value != null) {
+ if (value instanceof Integer) {
+ if (profile.getProperty(rootPropertyName) != null) {
Review comment:
What happen in case
propertyName = root.foo.bar
propertyTarget = bar
profile.root = {blabla:blabla}
event.bar = 15
the getNestedProperty will fail because the rootPropertyName will be "root" and the profile have the "root" property, but doesnt have the "root.foo".
To be verified, but I found the calculation and the usage of the "rootPropertyName" not clear, and hard to understand.
At the end it seem's only use to see if the profile have the property.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [unomi] jkevan edited a comment on pull request #265: UNOMI-446 improve increment action and make it generic
Posted by GitBox <gi...@apache.org>.
jkevan edited a comment on pull request #265:
URL: https://github.com/apache/unomi/pull/265#issuecomment-804209534
Small update on what we discussed about in the call:
- Do not touch the unomi IncrementInterestAction. This action have his own logic and cannot be generify with property incrementation, also the interest incrementation from JExperience is not using this action, it's actually using a script and the setPropertyAction.
- Create a new Action: IncrementPropertyAction:
- propertyName: to be able to increment or create a single prop (properties.pageViewCount)
- rootPropertyName: to be able to increment all props under this one: (ex: profile.properties.interest)
- incrementWithValues: to be able to increment properties with the values coming from the event. (ex: event.properties.interest)
- storeInSession: to be able to target the profile or the session.
### Real exemple coming from JEXperience:
the increment page count:
`"actions": [
{
"parameterValues": {
"setPropertyName": "properties.pageViewCount",
"setPropertyValue": "script::r = profile.properties['pageViewCount']; if (r == null) { profile.properties['pageViewCount'] = []; profile.properties.pageViewCount = [event.scope : 1] } else { if (r[event.scope] != null) { r[event.scope] = r[event.scope] + 1 } else { r[event.scope] = 1 }} r",
"storeInSession": false
},
"type": "setPropertyAction"
}
]`
We should be able to do it with this kind of conf:
type: incrementPropertyAction
paramerterValues:
- propertyName: properties.pageViewCount
- storeInSession: false
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [unomi] jkevan edited a comment on pull request #265: UNOMI-446 improve increment action and make it generic
Posted by GitBox <gi...@apache.org>.
jkevan edited a comment on pull request #265:
URL: https://github.com/apache/unomi/pull/265#issuecomment-804209534
### Small update on what we discussed about in the call:
- Do not touch the unomi IncrementInterestAction. This action have his own logic and cannot be generify with property incrementation, also the interest incrementation from JExperience is not using this action, it's actually using a script and the setPropertyAction.
- Create a new Action: IncrementPropertyAction:
- propertyName: to be able to increment or create a single prop (properties.pageViewCount)
- rootPropertyName: to be able to increment all props under this one: (ex: profile.properties.interest)
- incrementWithValues: to be able to increment properties with the values coming from the event. (ex: event.properties.interest)
- storeInSession: to be able to target the profile or the session.
### Real exemple coming from JEXperience:
**the increment page count:**
`"actions": [
{
"parameterValues": {
"setPropertyName": "properties.pageViewCount",
"setPropertyValue": "script::r = profile.properties['pageViewCount']; if (r == null) { profile.properties['pageViewCount'] = []; profile.properties.pageViewCount = [event.scope : 1] } else { if (r[event.scope] != null) { r[event.scope] = r[event.scope] + 1 } else { r[event.scope] = 1 }} r",
"storeInSession": false
},
"type": "setPropertyAction"
}
]`
We should be able to do it with this kind of conf:
**type: incrementPropertyAction
paramerterValues:
- propertyName: properties.pageViewCount
- storeInSession: false**
**The increment interest from jexperience:**
"actions": [
{
"parameterValues": {
"setPropertyName": "properties.interests",
"setPropertyValue": "script::r = profile.properties['interests']; foreach(interest : event.target.properties['interests'].entrySet()) { if (r == null) { r = [interest.key: interest.value] } else if (r[interest.key] != null) { r[interest.key] = r[interest.key] + interest.value } else { r[interest.key] = interest.value } } r",
"storeInSession": false
},
"type": "setPropertyAction"
}
]
We should be able to replace it, with:
**type: incrementPropertyAction
paramerterValues:
- rootPropertyName: properties.interest
- incrementWithEventValues: target.properties.interest
- storeInSession: false**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [unomi] jkevan commented on pull request #265: UNOMI-446 improve increment action and make it generic
Posted by GitBox <gi...@apache.org>.
jkevan commented on pull request #265:
URL: https://github.com/apache/unomi/pull/265#issuecomment-804209534
Small update on what we discussed about in the call:
- Do not touch the unomi IncrementInterestAction. This action have his own logic and cannot be generify with property incrementation, also the interest incrementation from JExperience is not using this action, it's actually using a script and the setPropertyAction.
- Create a new Action: IncrementPropertyAction:
- propertyName: to be able to increment or create a single prop (properties.pageViewCount)
- rootPropertyName: to be able to increment all props under this one: (ex: profile.properties.interest)
- incrementWithValues: to be able to increment properties with the values coming from the event. (ex: event.properties.interest)
- storeInSession: to be able to target the profile or the session.
Real exemple coming from JEXperience:
the increment page count:
`"actions": [
{
"parameterValues": {
"setPropertyName": "properties.pageViewCount",
"setPropertyValue": "script::r = profile.properties['pageViewCount']; if (r == null) { profile.properties['pageViewCount'] = []; profile.properties.pageViewCount = [event.scope : 1] } else { if (r[event.scope] != null) { r[event.scope] = r[event.scope] + 1 } else { r[event.scope] = 1 }} r",
"storeInSession": false
},
"type": "setPropertyAction"
}
]`
We should be able to do it with this kind of conf:
type: incrementPropertyAction
paramerterValues:
- propertyName: properties.pageViewCount
- storeInSession: false
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [unomi] Taybou merged pull request #265: UNOMI-446 improve increment action and make it generic
Posted by GitBox <gi...@apache.org>.
Taybou merged pull request #265:
URL: https://github.com/apache/unomi/pull/265
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org