[ 
https://issues.apache.org/jira/browse/CURATOR-609?focusedWorklogId=639040&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-639040
 ]

ASF GitHub Bot logged work on CURATOR-609:
------------------------------------------

                Author: ASF GitHub Bot
            Created on: 18/Aug/21 12:15
            Start Date: 18/Aug/21 12:15
    Worklog Time Spent: 10m 
      Work Description: Ryan0751 commented on a change in pull request #396:
URL: https://github.com/apache/curator/pull/396#discussion_r691178552



##########
File path: 
curator-x-async/src/main/java/org/apache/curator/x/async/modeled/details/ModeledCacheImpl.java
##########
@@ -188,9 +188,18 @@ private void internalChildEvent(TreeCacheEvent event)
         {
             ZPath path = ZPath.parse(event.getData().getPath());
             Entry<T> entry = entries.remove(path);
-            T model = (entry != null) ? entry.model : 
serializer.deserialize(event.getData().getData());
-            Stat stat = (entry != null) ? entry.stat : 
event.getData().getStat();
-            accept(ModeledCacheListener.Type.NODE_REMOVED, path, stat, model);
+            T model = null;
+            if (entry != null) {
+                model = entry.model;
+            }
+            else if (event.getData().getData() != null) {
+                model = serializer.deserialize(event.getData().getData());
+            }
+            if (model != null) {
+                Stat stat = (entry != null) ? entry.stat : 
event.getData().getStat();
+                accept(ModeledCacheListener.Type.NODE_REMOVED, path, stat, 
model);

Review comment:
       You are correct, this change would alter the behavior in that 
`accept(ModeledCacheListener.NODE_REMOVED... ` would not be called if the node 
in question doesn't contain any data (ie, a valid Model).
   
   However, I'm not sure this is a bad thing.  If you look above in the code at 
the implementation for the NODE_ADDED and NODE_UPDATE case, the `accept()` 
method is also not called if the ZNode in question does not contain any data.
   
   Unit testing doesn't seem to cover this use case either way (the tests all 
passed without any modification).




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 639040)
    Time Spent: 0.5h  (was: 20m)

> ModeledCache attempts to deserialize empty ZNodes on deletion, resulting in 
> exceptions
> --------------------------------------------------------------------------------------
>
>                 Key: CURATOR-609
>                 URL: https://issues.apache.org/jira/browse/CURATOR-609
>             Project: Apache Curator
>          Issue Type: Bug
>          Components: Framework
>    Affects Versions: 5.1.0
>            Reporter: Ryan Ruel
>            Priority: Major
>             Fix For: 5.2.1
>
>          Time Spent: 0.5h
>  Remaining Estimate: 0h
>
> When using the ModeledCache implementation, when a ZPath is deleted the 
> implementation assumes that all ZNodes in the path contain a valid instance 
> of the Model.
> If intermediary paths have been created that do not contain a Model, the 
> implementation is calling the Jackson deserialize method with null data, 
> resulting in an exception.
> Instead, the code should ensure there is valid data present prior to 
> attempting to deserialize and call the "accept()" (callback) method on action 
> NODE_REMOVED.
> Example:
> TestModel child1 = new TestModel("d", "e", "f", 1, BigInteger.ONE);
>  ZPath path1 = path.child("foo").child("bar").child("child1");
> try (CachedModeledFramework<TestModel> client = ModeledFramework.wrap(async, 
> modelSpec).cached())
> {    CountDownLatch latch = new CountDownLatch(1);    
> client.listenable().addListener((t, p, s, m) -> latch.countDown());    
> client.start();    complete(client.withPath(path1).set(child1));    
> assertTrue(timing.awaitLatch(latch));    assertDoesNotThrow(() -> 
> rawClient.delete().deletingChildrenIfNeeded().forPath(path.toString())); }
> After calling "delete()", an exception is thrown:
> ERROR 
> org.apache.curator.x.async.modeled.TestCachedModeledFramework$$Lambda$409/0x0000000800e0c850
>  Could not process cache message [Curator-SafeNotifyService-0]
>  java.lang.IllegalArgumentException: argument "src" is null
>  at 
> com.fasterxml.jackson.databind.ObjectReader._assertNotNull(ObjectReader.java:2120)
>  
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to