[ 
https://issues.apache.org/jira/browse/SLING-5494?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15137063#comment-15137063
 ] 

Timothee Maret commented on SLING-5494:
---------------------------------------

The implementation does behave the opposite of what the API tells regarding the 
{{propertyChanged}} parameter.
I suggest to change the API description in order to match the implementation 
because:

1. The implementation semantic of the {{propertyChanged}} is "logical" (if 
{{true}} keep the properties that have changed)
2. Assuming there are existing users of the API, it is unlikely that the API 
behaviour has been relied upon rather than the implementation behaviour (the 
difference in behaviour is obvious).

> Discovery InstancesDiff.retained() implementation not according to API doc
> --------------------------------------------------------------------------
>
>                 Key: SLING-5494
>                 URL: https://issues.apache.org/jira/browse/SLING-5494
>             Project: Sling
>          Issue Type: Bug
>          Components: Extensions
>    Affects Versions: Discovery Commons 1.0.4
>            Reporter: Marc Pfaff
>
> The InstancesDiff.retained(boolean retainFromNewCollection, boolean 
> propertyChanged) does not seem to be implemented according to the API docs. 
> The property 'propertyChanged' is implemented the opposite way as in the API 
> descriptions. Not sure whether implementation or API doc is right. 
> As per API the propertyChanged=false is supposed to return only the instances 
> where properties have changed, and propertyChanged=true where properties have 
> not changed.
> Looking at the test case in [3], which is passing, the implementation seems 
> to behave the other way round. 
> * Suspect implementation [2]
> * Test case [3]
> [1] 
> https://github.com/apache/sling/blob/trunk/bundles/extensions/discovery/commons/src/main/java/org/apache/sling/discovery/commons/InstancesDiff.java#L214
> [2] 
> https://github.com/apache/sling/blob/7c4a53755aed1211c9af313a3973cd2543a7bbe0/bundles/extensions/discovery/commons/src/main/java/org/apache/sling/discovery/commons/InstancesDiff.java#L256
> [3] 
> https://github.com/apache/sling/blob/trunk/bundles/extensions/discovery/commons/src/test/java/org/apache/sling/discovery/commons/InstancesDiffTest.java#L161



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to