ACS-10772 Fix for cm:modifiedAt property not in sync between ACS & ElasticSearch #3748
Conversation
|
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
repository/src/test/java/org/alfresco/repo/event2/EventConsolidatorUnitTest.java
Fixed
Show fixed
Hide fixed
repository/src/test/java/org/alfresco/repo/event2/EventConsolidatorUnitTest.java
Fixed
Show fixed
Hide fixed
repository/src/test/java/org/alfresco/repo/event2/EventConsolidatorUnitTest.java
Fixed
Show fixed
Hide fixed
|
|
|
|
SatyamSah5
requested review from
bsayan2,
damianujma,
jakubkochman and
kushal-banik-hyland
repository/src/main/java/org/alfresco/repo/event2/NodeEventConsolidator.java
Outdated
Show resolved
Hide resolved
repository/src/main/java/org/alfresco/repo/event2/NodeEventConsolidator.java
Outdated
Show resolved
Hide resolved
repository/src/main/java/org/alfresco/repo/event2/NodeEventConsolidator.java
Outdated
Show resolved
Hide resolved
|
|
repository/src/main/java/org/alfresco/repo/event2/NodeEventConsolidator.java
Outdated
Show resolved
Hide resolved
repository/src/main/java/org/alfresco/repo/event2/NodeEventConsolidator.java
Outdated
Show resolved
Hide resolved
|
You wrote |
repository/src/main/java/org/alfresco/repo/event2/NodeEventConsolidator.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| Map<String, Map<String, String>> localizedProps = helper.getLocalizedPropertiesBefore(changedPropsBefore, after); | ||
| Map<String, Map<String, String>> localizedProps = helper.getLocalizedPropertiesBefore(propertiesBefore, after); |
There was a problem hiding this comment.
This was the reason for mismatch in Share and ADW behavior. The getLocalizedPropertiesBefore method
public Map<String, Map<String, String>> getLocalizedPropertiesBefore(Map<QName, Serializable> propsBefore, NodeResource nodeAfter)
expects us to pass node properties before update but we were actually passing changedPropsBefore which didn't contained all the previous properties but only the properties that got modified.
|
|
repository/src/main/java/org/alfresco/repo/event2/NodeEventConsolidator.java
Outdated
Show resolved
Hide resolved
|
|
|
|
|
|
|
|
||
| Map<String, Map<String, String>> localizedProps = helper.getLocalizedPropertiesBefore(changedPropsBefore, after); | ||
| Map<String, Map<String, String>> localizedProps = helper.getLocalizedPropertiesBefore(propertiesBefore, after); | ||
| if (!localizedProps.isEmpty()) |
There was a problem hiding this comment.
I am thinking about the flow, I believe the getBeforeMapChanges returns the properties like
(SET A - SET B) U (null values for all (SET B- SET A)). So basically all the changed values will be there. Now If we consider getLocalizedPropertiesBefore this will check for MLtexts changed. (subset of changes). This could be determined by getLocalizedPropertiesBefore(changedPropsBefore, after)
Rest things can be filtered out by the property mapToNodeLocalizedProperties was set to null and others unmapped. The sets union now calculates the same keys which are changed in exact QName MLtext change which can be determined by their values (ig thats why its wrapped under this block
if (!changedPropsBefore.isEmpty()) means there is a chance or not for having changed values. So my question is.
- Why current code is not working for ``getLocalizedPropertiesBefore(changedPropsBefore, after)` ?
- Why its working for ADW not Share?
Maybe I have overlooked something but the confusion is around this mostly
There was a problem hiding this comment.
For this scenario let's say for a node only modifiedAt changes, so changedPropsBefore would only have modifiedAt entry. Now if you pass this to getLocalizedPropertiesBefore, it compares the before and after localized properties, now for after we are passing NodeResource after which has all the properties to compare and in our scenario it has empty title and description. Now in getLocalizedPropertiesBefore method, while comparing the before and after, before being changedPropsBefored it does not have entries for title and description as it was not changes, so it gets null for those properties and after has the empty string. So, code identifies it as a change and return title and description as entries.
Whereas now, if we pass propertiesBefore it has all the previous properties and not the properties that are changed. So for comparing title and description, it would get empty string for both before and after, hence would not find any differences.
It is little complex to explain through words :(
There was a problem hiding this comment.
In case of ADW, title and description get set to null if we dont provide any and similarily with changedPropsBefore you would get title and description as null, so both before and after would be null resulting in logic identifying no changes in the property.
There was a problem hiding this comment.
Got it! it was a confusion between union and intersection. Thanks :)
|
|
SatyamSah5
changed the title
|
|
|
|
damianujma
left a comment
damianujma
left a comment
There was a problem hiding this comment.
LGTM!
Please update the license headers.
|
|
For a node, if only
modifiedAtproperty is updated, we were filtering out the node update events for such scenarios, which led to mismatch inmodifiedAtbetween ACS and ElasticSearch for that node. Have added logic to allow event generation in such scenarios as well.