Unsorted results when using ReadThroughMultiCacheAdvice

[GoogleCodeExporter]

Hi,

We are using ReadThroughMultiCacheAdvice and if the results are partially in 
the cache, the output of the method isn't sorted correctly as it doesn't 
respect the order of the keys passed in argument.

The problem is in generateByKeysFromResult and generateByKeysProviders: the 
results should be sorted using the keys order in these methods.

Did we miss something or do you agree there is a bug here?

Thanks for your feedback.

-- 
Guillaume

Original issue reported on code.google.com by guillaum...@gmail.com on 28 Nov 2013 at 2:11

[GoogleCodeExporter]

What version of SSM do you use? 

Original comment by ragno...@gmail.com on 29 Nov 2013 at 6:15

[GoogleCodeExporter]

We use 3.2.1.

You can see one side of the problem here: 
http://code.google.com/p/simple-spring-memcached/source/browse/trunk/simple-spri
ng-memcached/src/main/java/com/google/code/ssm/aop/ReadThroughMultiCacheAdvice.j
ava#161

You add the cached results at the end of the list so you end up with a list of:
- results from the method invocation
- results from the cache

instead of having the results sorted in parameters order.

We are willing to dedicate time to fix it but I think we should first discuss 
how you think it would be nice to fix it.

Original comment by guillaum...@gmail.com on 29 Nov 2013 at 7:54

[GoogleCodeExporter]

You're right. I'll think about the issue and how it can be fixed without 
decreasing performance. 

If you have already an idea how to improve the code to solve the issue please 
let me know.

Original comment by ragno...@gmail.com on 29 Nov 2013 at 4:56

[GoogleCodeExporter]

I've reviewed the code and in case of generateByKeysProviders method the issue 
doesn't occur if an underlying method returns list that size is the same as the 
size of input list (one of the method's argument of type List annotated with 
ParameterValueKeyProvider). It works correctly because 
MultiCacheCoordinator.generateResultList uses the input list to define order of 
elements in result list. 
If an underlying method returns less elements than requested by input list 
argument (i.e. in case when the method skips null values in results) no 
correlations/mappings can be assumed between the input list and returned result 
and result returned by SSM is also unsorted. 

Do you agree?

Original comment by ragno...@gmail.com on 9 Dec 2013 at 1:52

[GoogleCodeExporter]

Yes, I agree.

I think that you can't have a fast path even if it looks appealing and you're 
forced to add all the objects to the coord before generating the full list.

To fix generateByKeysFromResult (this is the one we use in our app), I think 
you need something along the lines of:
Index: src/main/java/com/google/code/ssm/aop/ReadThroughMultiCacheAdvice.java
===================================================================
--- 
src/main/java/com/google/code/ssm/aop/ReadThroughMultiCacheAdvice.java  (revision
 514)
+++ 
src/main/java/com/google/code/ssm/aop/ReadThroughMultiCacheAdvice.java  (working 
copy)
@@ -151,6 +151,7 @@
                 getCacheBase().getCache(coord.getAnnotationData()).setSilently(cacheKey, data.getExpiration(), resultObject,
                         serializationType);
                 coord.getMissedObjects().remove(coord.getKey2Obj().get(cacheKey));
+                coord.getKey2Result().put(cacheKey, resultObject);
             }
         }

@@ -158,8 +159,7 @@
             addNullValues(coord.getMissedObjects(), coord, serializationType);
         }

-        results.addAll(coord.generatePartialResultList());
-        return results;
+        return coord.generateResultList();
     }

     private List<?> generateByKeysProviders(final List<Object> results, final MultiCacheCoordinator coord,

I haven't exactly understood the first case in generateByKeysProviders so I 
don't know what is the best way to fix it in this case.

Original comment by guillaum...@gmail.com on 9 Dec 2013 at 5:52

[GoogleCodeExporter]

After giving some more thoughts, I think generateByKeysProviders can't be fixed 
and we should just add a warning about the ordering in the javadoc.

I still think it's worth it to fix generateByKeysFromResult as described above.

Original comment by guillaum...@gmail.com on 10 Dec 2013 at 9:28

[GoogleCodeExporter]

May I know why do you use 
ReadThroughMultiCacheOption(generateKeysFromResult=true)? As you see in javadoc 
the purpose of the generateKeysFromResult is to add objects to the cache even 
if amount of elements returned by underlying method is different than amount of 
IDs in list method's argument. But you're right, even in such case the order of 
results can be fixed.

Original comment by ragno...@gmail.com on 11 Dec 2013 at 8:24

• Changed state: Accepted

[GoogleCodeExporter]

Fix is available in trunk.

Original comment by ragno...@gmail.com on 11 Dec 2013 at 1:52

• Changed state: Ready

[GoogleCodeExporter]

Original comment by ragno...@gmail.com on 11 Dec 2013 at 1:52

• Added labels: Milestone-Release3.3.0

[GoogleCodeExporter]

To answer your question, we thought it was more robust to generate the keys of 
the object.

Thanks for the fix.

Original comment by guillaum...@gmail.com on 11 Dec 2013 at 3:46

[GoogleCodeExporter]

Hi,

Do you have any plan for a release?

Thanks!

Original comment by guillaum...@gmail.com on 16 Jan 2014 at 10:36

[GoogleCodeExporter]

Hi,

I'll try to release SSM today if not then in a next week.

Original comment by ragno...@gmail.com on 17 Jan 2014 at 12:47

[GoogleCodeExporter]

Included in release 3.3.0. The release is available in central maven 
repository. 

Original comment by ragno...@gmail.com on 17 Jan 2014 at 6:16

• Changed state: Fixed