[jira] [Commented] (JCRVLT-335) Remove AggregateImpl cast in DocViewSerializer

["Tobias Bocanegra (JIRA)" <ji...@apache.org>]

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