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 2020/12/19 17:53:51 UTC

[GitHub] [unomi] liatiusim opened a new pull request #229: Bug fix in profile property condition and add getNestedProperty method

liatiusim opened a new pull request #229:
URL: https://github.com/apache/unomi/pull/229


   When querying using "exist" operator in Condition, an existQuery is created and sent to Elasticsearch. According to ES documentation, exist return false the the field in null or empty list - "[]".
   
   The case of an empty list wasn't handled in the eval() method of profile property condition evaluator. Need to return false if the field exist but has a value which is empty list.


----------------------------------------------------------------
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 merged pull request #229: UNOMI-411 Bug fix in profile property condition and add getNestedProperty method

Posted by GitBox <gi...@apache.org>.
sergehuber merged pull request #229:
URL: https://github.com/apache/unomi/pull/229


   


----------------------------------------------------------------
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 pull request #229: UNOMI-411 Bug fix in profile property condition and add getNestedProperty method

Posted by GitBox <gi...@apache.org>.
sergehuber commented on pull request #229:
URL: https://github.com/apache/unomi/pull/229#issuecomment-758633270


   Could you also implement an integration test for this?


----------------------------------------------------------------
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] liatiusim commented on a change in pull request #229: UNOMI-411 Bug fix in profile property condition and add getNestedProperty method

Posted by GitBox <gi...@apache.org>.
liatiusim commented on a change in pull request #229:
URL: https://github.com/apache/unomi/pull/229#discussion_r563548668



##########
File path: api/src/main/java/org/apache/unomi/api/Profile.java
##########
@@ -94,6 +95,30 @@ public Object getProperty(String name) {
         return properties.get(name);
     }
 
+    /**
+     * Retrieves the value of the nested property identified by the specified name.
+     *
+     * @param name the name of the property to be retrieved, splited in the nested properties with "."
+     * @return the value of the property identified by the specified name
+     */
+    public Object getNestedProperty(String name) {
+        if (!name.contains(".")) {

Review comment:
       Thank you for the comment. I'm not sure if I know where to put the common code, what do you think?




----------------------------------------------------------------
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 #229: Bug fix in profile property condition and add getNestedProperty method

Posted by GitBox <gi...@apache.org>.
sergehuber commented on a change in pull request #229:
URL: https://github.com/apache/unomi/pull/229#discussion_r552420053



##########
File path: api/src/main/java/org/apache/unomi/api/Profile.java
##########
@@ -94,6 +95,30 @@ public Object getProperty(String name) {
         return properties.get(name);
     }
 
+    /**
+     * Retrieves the value of the nested property identified by the specified name.
+     *
+     * @param name the name of the property to be retrieved, splited in the nested properties with "."
+     * @return the value of the property identified by the specified name
+     */
+    public Object getNestedProperty(String name) {
+        if (!name.contains(".")) {

Review comment:
       I'm wondering if there is a way we could avoid this code duplication but I don't know where to put it unless we create some kind of static class in the API. 




----------------------------------------------------------------
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] liatiusim commented on a change in pull request #229: UNOMI-411 Bug fix in profile property condition and add getNestedProperty method

Posted by GitBox <gi...@apache.org>.
liatiusim commented on a change in pull request #229:
URL: https://github.com/apache/unomi/pull/229#discussion_r563548668



##########
File path: api/src/main/java/org/apache/unomi/api/Profile.java
##########
@@ -94,6 +95,30 @@ public Object getProperty(String name) {
         return properties.get(name);
     }
 
+    /**
+     * Retrieves the value of the nested property identified by the specified name.
+     *
+     * @param name the name of the property to be retrieved, splited in the nested properties with "."
+     * @return the value of the property identified by the specified name
+     */
+    public Object getNestedProperty(String name) {
+        if (!name.contains(".")) {

Review comment:
       Thank you for the comment. I'm not sure if I know where to put the common code, what do you think?




----------------------------------------------------------------
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] liatiusim commented on pull request #229: UNOMI-411 Bug fix in profile property condition and add getNestedProperty method

Posted by GitBox <gi...@apache.org>.
liatiusim commented on pull request #229:
URL: https://github.com/apache/unomi/pull/229#issuecomment-774620119


   Added integration tests


----------------------------------------------------------------
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 #229: UNOMI-411 Bug fix in profile property condition and add getNestedProperty method

Posted by GitBox <gi...@apache.org>.
sergehuber commented on a change in pull request #229:
URL: https://github.com/apache/unomi/pull/229#discussion_r573682531



##########
File path: api/src/main/java/org/apache/unomi/api/Profile.java
##########
@@ -94,6 +95,30 @@ public Object getProperty(String name) {
         return properties.get(name);
     }
 
+    /**
+     * Retrieves the value of the nested property identified by the specified name.
+     *
+     * @param name the name of the property to be retrieved, splited in the nested properties with "."
+     * @return the value of the property identified by the specified name
+     */
+    public Object getNestedProperty(String name) {
+        if (!name.contains(".")) {

Review comment:
       Yeah let's leave it like this for the time being but we'll probably have to refactor at some point.




----------------------------------------------------------------
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