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