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

Konrad Windszus commented on SLING-12742:
-----------------------------------------

[~cziegeler]
Can you share exactly the code path which leads to a new exception and 
backwards-compatibility issues in your PoV?

I introduced a new exception in the private method {{createValue(...)}}. This 
one is called from 
# 
https://github.com/apache/sling-org-apache-sling-jcr-resource/blob/e087df12164667b6746370c914d78718ed5568bb/src/main/java/org/apache/sling/jcr/resource/internal/helper/JcrPropertyMapCacheEntry.java#L101C25-L101C42
 which was already throwing an exception before in that case (although a 
different one) and is only called from one constructor. However the only caller 
of that constructor is already wrapping the RepositoryException in an 
IllegalStateException in 
https://github.com/apache/sling-org-apache-sling-jcr-resource/blob/e087df12164667b6746370c914d78718ed5568bb/src/main/java/org/apache/sling/jcr/resource/internal/JcrModifiableValueMap.java#L113
 -> so same ISE thrown before and after my change (only difference is a longer 
stacktrace and wrapped exception)
# 
https://github.com/apache/sling-org-apache-sling-jcr-resource/blob/e087df12164667b6746370c914d78718ed5568bb/src/main/java/org/apache/sling/jcr/resource/internal/helper/JcrPropertyMapCacheEntry.java#L403
 which is called from the private methods {{convertToType}} and 
{{convertInputStream}} which are transitively only called from 
https://github.com/apache/sling-org-apache-sling-jcr-resource/blob/e087df12164667b6746370c914d78718ed5568bb/src/main/java/org/apache/sling/jcr/resource/internal/helper/JcrPropertyMapCacheEntry.java#L237
 which catches and logs the exception. Also here I fail to see how an external 
caller would ever see a difference: No exception either before and after my 
patch.

[~angela] In general I appreciate timely reviews (which didn't happen here) 
although I know that sometimes this is not possible, however we don't have a 
[RTC policy|https://www.apache.org/foundation/glossary.html#ReviewThenCommit] 
in Sling. In case you propose to change that please bring it to the mailing 
list.



> Don't swallow java.io.NotSerializableException in 
> JcrPropertyMapCacheEntry.createValue()
> ----------------------------------------------------------------------------------------
>
>                 Key: SLING-12742
>                 URL: https://issues.apache.org/jira/browse/SLING-12742
>             Project: Sling
>          Issue Type: Improvement
>          Components: JCR
>    Affects Versions: JCR Resource 3.3.2
>            Reporter: Konrad Windszus
>            Assignee: Konrad Windszus
>            Priority: Major
>             Fix For: JCR Resource 3.3.4
>
>
> Currently a log like this is exposed in case a class cannot be serialized 
> (despite having the marker interface Serializable) when an object of that 
> type is put into a {{ModifiableValueMap}}:
> {code}
> Caused by: java.lang.IllegalArgumentException: Value can't be stored in the 
> repository: <my object>
>       at 
> org.apache.sling.jcr.resource.internal.helper.JcrPropertyMapCacheEntry.failIfCannotStore(JcrPropertyMapCacheEntry.java:109)
>  [org.apache.sling.jcr.resource:3.3.2]
>       at 
> org.apache.sling.jcr.resource.internal.helper.JcrPropertyMapCacheEntry.<init>(JcrPropertyMapCacheEntry.java:97)
>  [org.apache.sling.jcr.resource:3.3.2]
>       at 
> org.apache.sling.jcr.resource.internal.JcrModifiableValueMap.put(JcrModifiableValueMap.java:63)
>  [org.apache.sling.jcr.resource:3.3.2]
> {code}
> The underlying exception was a 
> {code}
> java.io.NotSerializableException: java.util.Optional
> {code}
> which was swallowed in 
> https://github.com/apache/sling-org-apache-sling-jcr-resource/blob/7d980aa423bf32b639bea5c767d1fe6ec66773b7/src/main/java/org/apache/sling/jcr/resource/internal/helper/JcrPropertyMapCacheEntry.java#L135



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to