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

Tobias Bocanegra commented on JCRVLT-335:
-----------------------------------------

of course you are correct. The project grew quite unstructured over the last 10 
years, so a lot of the API isn't correct :-)
Unfortunately, I don't have time to rewrite it, not can we break backward 
compatibility of the public API....

The best we can do, is to check the class of the parameter and try to work 
around if it's not an internal one.

> Remove AggregateImpl cast in DocViewSerializer 
> -----------------------------------------------
>
>                 Key: JCRVLT-335
>                 URL: https://issues.apache.org/jira/browse/JCRVLT-335
>             Project: Jackrabbit FileVault
>          Issue Type: Improvement
>          Components: vlt
>            Reporter: Radosław Wesołowski
>            Priority: Major
>             Fix For: 3.2.6
>
>
>  The 
> [cast|https://github.com/apache/jackrabbit-filevault/blob/1931b593de726e42ead33aac12b8ff854371d343/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewSerializer.java#L48]
>  of 
> [Aggregate|https://github.com/apache/jackrabbit-filevault/blob/1931b593de726e42ead33aac12b8ff854371d343/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/api/Aggregate.java]
>  to 
> [AggregateImpl|https://github.com/apache/jackrabbit-filevault/blob/1931b593de726e42ead33aac12b8ff854371d343/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/AggregateImpl.java]
>  in 
> [DocViewSerializer|https://github.com/apache/jackrabbit-filevault/blob/1931b593de726e42ead33aac12b8ff854371d343/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewSerializer.java]'s
>  constructor as in
> {code:java}
> public DocViewSerializer(Aggregate aggregate) {
>       this.aggregate = (AggregateImpl) aggregate;
> }
> {code}
> is breaking at least the [Liskov substitution 
> principle|https://en.wikipedia.org/wiki/Liskov_substitution_principle]. Any 
> client of such class would be surprised to see it break upon passing a 
> seemingly correct parameter of type 
> [Aggregate|https://github.com/apache/jackrabbit-filevault/blob/1931b593de726e42ead33aac12b8ff854371d343/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/api/Aggregate.java].
>  The least that could be done would be to replace the parameter type to 
> [AggregateImpl|https://github.com/apache/jackrabbit-filevault/blob/1931b593de726e42ead33aac12b8ff854371d343/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/AggregateImpl.java].
>  In the current form the design has a definitive code smell unfortunately.
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to