Usage of Spring EL for a new CacheKeyGenerator

[GoogleCodeExporter]

Hi,

Nice project! 
I did not find another way of contacting anyone and cannot set the issue type 
to suggestion or something alike so here goes:

I somewhat coded a simular project for caching using Spring and EHCache. I am 
using a specific property in the annotation for retrieval of the cacheKeys in 
the arguments of the method.
E.g.:
@Cacheable(argSpELToCacheId = { "id", "id2" })

The arguments are traversed using Spring EL and thus a way of getting the 
cacheKey.

Any tips on integrating this into your code? I have a working version and here 
is my logic:
1.alterring @Cacheable, CacheableAttribute(and Impl), CacheAttributeSourceImpl 
with the new property
2. The EHCacheInterceptor to pass the actual values to the CacheKeyGenerators 
and call new method see 3.
3. adding new method in CacheKeyGenerator T generateKey(MethodInvocation 
methodInvocation, MethodAttribute methodAttribute); Of course this is just a 
hack to get it to work.
4.Modify AbstractCacheKeyGenerator to implement this method Actually now 
replacing the one without the extra parameter but to not break everything I 
opted to do this for now. Forcing myself to use 
includeMethod,includeParameterTypes so I only need to alter 1 method
5. Creating SpELCacheKeyGenerator and is working.
Maybe a good idea to put the concept of identifying your own cache-key like 
this (see JPA/Hibernate @Id) into  next version.
Maybe put these EL values in the @KeyGenerator annotation and then passing it 
to the Generators when generating the key, instead of now using the @Cacheable 
for this? 
What do you think?
Anyway thanks and keep up the good wok!

Kind regards,
Timothy

Original issue reported on code.google.com by timothy....@gmail.com on 13 Aug 2010 at 2:57

[GoogleCodeExporter]

This seems like a neat idea, if you would be willing to post a patch showing 
what you have so far we could work on getting it incorporated and included in 
the next release,

Original comment by eric.dalquist on 15 Sep 2010 at 8:22

• Added labels: Milestone-Release2.0.0, Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

Meant to tag it for 1.2, not 2.0

Original comment by eric.dalquist on 15 Sep 2010 at 8:23

• Added labels: Milestone-Release1.2.0
• Removed labels: Milestone-Release2.0.0

[GoogleCodeExporter]

Hi,

I am plaesed to share my code but do not know how to create a patch. Tried 
Eclipse > Team >Create Patch >... but keep 

saying org.tigris.subversion.javahl.ClientException: Operation not permitted
svn: Can't move 
'/Applications/java/ide/workspaces/workspace-flow/ehcache-spring-annotations/src
/test/java/com/.svn/tmp/entries' to 
'/Applications/java/ide/workspaces/workspace-flow/ehcache-spring-annotations/src
/test/java/com/.svn/entries': Operation not permitted

How can I share my code? Can I commit it to a branch or something? Or just zip 
it and provide it as an attachement?

Kind regards,
Timothy

Original comment by timothy....@gmail.com on 16 Sep 2010 at 6:00

[GoogleCodeExporter]

Strange error.

You can go to the root of the project on the command line and run:
svn diff > my.patch

That produces a patch file in the correct format, just attach that and we'll 
look at the getting it incorporated.

Original comment by eric.dalquist on 17 Sep 2010 at 12:20

• Changed state: Accepted

[GoogleCodeExporter]

Hi,

it worked at command line and I attached it to this comment. I created a unit 
test as well to see how it is expected to be used. 
It has been a while since I worked on this and I am currently not using it in 
prod because other more urgent things needed my attention. So lets say it was a 
work in progress ;-) 

Timothy

Original comment by timothy....@gmail.com on 17 Sep 2010 at 3:34

Attachments:

• my.patch

[GoogleCodeExporter]

Thanks! I'm pretty busy right now but should have a chance to take a look at it 
early next week. I'll be sure to let you know when I do.

Original comment by eric.dalquist on 17 Sep 2010 at 4:27

[GoogleCodeExporter]

Hi, 
a SpEL Keybuilder would be great for my current project. I volunteer to write 
the code and tests. 

Instead of @Cacheable I would recommend to extend @KeyGenerator to fit to the 
other generators. for example:

Cacheable(cacheName="weather", 
    keyGenerator = @KeyGenerator (
            name = "SpELKeyGenerator",
            properties = {
                    @Property( name="expression", value=""TODO" ),
            }
        )
    )

Original comment by peters.a...@gmail.com on 19 Sep 2010 at 11:36

[GoogleCodeExporter]

Hi,

like mentioned in first post I also agree on the part to add it to the 
KeyGenerator
Like I also said a work in progress because in my code there is no way to 
distinguish the EL for which parameter. There needs to be some way of marking 
the EL and the exact parameter. I currently just have an array of 
EL=expressions: first expression is for the first parameter and second is for 
the second,...
Maybe something in the lines of:

@Cacheable(cacheName="cacheName", 
    keyGenerator = @KeyGenerator (
            name = "SpELKeyGenerator",
            properties = {
                    @Property( name="expressionForParam1", value="id" ),
                    @Property( name="expressionForParam2", value="name" ),
            }
        )
    )
public String get(Model m, Model2 m2){
         return ...
}

public class Model{
 private String id;
 private String somethingElse;
...
}

public class Model2{
   private String id;
   private String name;
 ...
}

Which would mean take the 'id' of the first parameter(Model) and the 'name' of 
parameter 2(Model2) for the cachekey.

Or maybe some custom interpreting if only 1 @Property can be defined
@Cacheable(cacheName="cacheName", 
    keyGenerator = @KeyGenerator (
            name = "SpELKeyGenerator",
            properties = {
                    @Property( name="expression", value="{id},{name}" )
            }
        )
    )

For multiple cacheKeys on the same parameter something like this:
@Cacheable(cacheName="cacheName", 
    keyGenerator = @KeyGenerator (
            name = "SpELKeyGenerator",
            properties = {
                    @Property( name="expressionForParam1", value="id,somethingElse" ),
                    @Property( name="expressionForParam2", value="name" ),
            }
        )
    )

or only 1 expression:

@Cacheable(cacheName="cacheName", 
    keyGenerator = @KeyGenerator (
            name = "SpELKeyGenerator",
            properties = {
                    @Property( name="expression", value="{id,somethingElse},{name}" )
            }
        )
    )

Of course 3 parameters but only p1 and p3 must be used... that is why by 
pointing to the parameter is slightly better :-) Look at Spring Constructor 
injection Argument Index but then how to use it in @Property… Anyway still 
lots of stuff to think about.

My current project is getting finalized and the for improved performance I can 
work on this as well.

Let me know how it is going down.

Greetings,
T

Original comment by timothy....@gmail.com on 19 Sep 2010 at 3:53

[GoogleCodeExporter]

I haven't had a chance to look at the patch yet but my thought was we could 
simply pass in the MethodInvocation as the SpEL context. Then your expression 
could look like:

"{arguments[0].id, arguments[0].someFiels, arguments[1].name}"

That's a simple example, essentially you just return some sort of Object from 
your expression that can operate on the entire MethodInvocation.

I was also thinking that you would specify another CacheKeyGenerator instance 
to do the actual key generation. That would let you use and of the existing key 
format strategies as well as use the reflection based generation if needed. We 
should be able to do all of this without modifying the annotations:

@Cacheable(cacheName="cacheName", 
  keyGenerator = @KeyGenerator (
    name = "SpELKeyGenerator",
    properties = {
      @Property( name="expression", value="{arguments[0].id, arguments[0].someFiels, arguments[1].name}" )
      @Property( name="keyGenerator", value="StringCacheKeyGenerator")
            }
        )
    )

Original comment by eric.dalquist on 19 Sep 2010 at 4:46

[GoogleCodeExporter]

the MethodInvocation contains all information but i would recommend to focus on 
the named parameters. no type or method information as in the other builders. 
just rely on having a matching expression for this annotated method. 

a composite key can be build with mathematical operators, for example:
param1.id*1000000+param2.id

or by registered functions based on current keybuilders like:
hash(param1.id, param2.name, param3.hash())
digest(param1.something, param2.name)
digest('test'+param1.something+  param2.name)

with a single parameter method the name can be skipped.
hash(name)

Original comment by peters.a...@gmail.com on 19 Sep 2010 at 5:04

[GoogleCodeExporter]

sorry, we don't have named parameters ;-)
replace the samples with arguments[0]..[1]

Original comment by peters.a...@gmail.com on 19 Sep 2010 at 5:18

[GoogleCodeExporter]

Hi,

Quickly changed code to see if this could work and it does. Got is working 
using the SpEL arguments[index]. ... Great although now using a new class that 
contains the arguments-property so the SpEL can get its value. Creating an new 
object for each generated key is a bit overkill and slows down performance so 
Erics idea to pass the MethodInvocation should solve this (but this would 
require a bit more time than I have at the moment)

Additionally instead of mathematical operations for cache key generation I 
opted for the actual generating using the full name (including packages) 
returnType + full Classname of the cahced class + method name + full 
parameterTypeName=arg0VALUE+patameterType2=arg1VALUE,... Because there is a 
possibility that by using mathematical calculation 2 cached methods could 
return same cacheKey when using the same cache. Anyway the initial code was a 
work in progress so I could be wrong.

I attached the new patch here as well.

Greets,
T

Original comment by timothy....@gmail.com on 20 Sep 2010 at 8:09

Attachments:

• my.patch2

[GoogleCodeExporter]

thanks for your patch! i'll look at it later as i'm currently busy with company 
work. 
I would be very glad having the method arguments in any way. the rest like 
classname, return type etc still seems unnecessary in my cases because i could 
always modify my SpEL expression on each method instead of having a catch all 
expression. but maybe anyone else has better usecases.

for example:
# hash(arg[0], 'weatherLong')
public Weather getWeather(Long locationId);

# hash(arg[0], 'weatherzip', 'classB')
public Weather getWeather(String zipCode);

Original comment by peters.a...@gmail.com on 20 Sep 2010 at 9:22

[GoogleCodeExporter]

Got it working using @Property and it changes nothing to the initial code 
checked out from svn. Will post patch later. Really great!

Original comment by timothy....@gmail.com on 20 Sep 2010 at 6:02

[GoogleCodeExporter]

Hi,

No need to look at patch because I saw that the patch did not contain any new 
files so now I will attach the sources and tests (which are all new files and 
no change to the initial code checked out from svn) as a zip. Copy paste in 
correct packages...

There are 2 versions of a SpELKeyGenerator. 1 that works and 1 that works 
almost 2 times faster because of initialization done... however because of the 
auto generated spring-bean-name there can be a collision see source for details 
and a workaround:

TODO= Either change creation of unique bean-name generation because of the
  possibility of collision. E.g.: when the same expression is used for 2
  methods but different argument types it will use the same instance and since
  it probably is a Singleton it will use the same initialized properties (which
  is incorrect) 
  OR
  workaround use another dummy @Property with a unique key-value since the
  generation of the beanname is based upon all the parameters

Anyway, let me know what you think once you have the time to look at it.

Greetings,
T

Original comment by timothy....@gmail.com on 20 Sep 2010 at 11:16

Attachments:

• googlecode.ehcache.zip

[GoogleCodeExporter]

Original comment by eric.dalquist on 25 Oct 2010 at 2:34

• Changed state: Started

[GoogleCodeExporter]

I just committed a first pass at SpEL cache key generation:
http://code.google.com/p/ehcache-spring-annotations/source/detail?r=587

It exposes the argument Object[] as #args for convenience and the original 
MethodInvocation and #invocation. Also all of the built in key generators are 
available via a special #key object so you can do things like:
#key.digest(args[2].someProperty, args[1])

And get the MessageDigest output of those two values.

I haven't done extensive performance testing but at first glance the SpEL 
evaluation appears to add about 10% to the key generation time (using #key.hash 
for my test). I'd expect that percentage to be much lower if a more expensive 
key generator is used in the SpEL (such a reflective digesting: #key.digestR)

I'll run the code through a profiler this week and work on the documentation 
page for the key generator. Please take a look at the class and feel free to 
provide your thoughts and feedback.

Original comment by eric.dalquist on 26 Oct 2010 at 4:25

[GoogleCodeExporter]

BTW: The test relies on a given TimeZone and fails i my default tz:

Tests run: 9, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.19 sec <<< 
FAILURE!
testComplexHashCode(com.googlecode.ehcache.annotations.key.SpELCacheKeyGenerator
Test)  Time elapsed: 0.003 sec  <<< FAILURE!
org.junit.ComparisonFailure: expected:<...alse, true], [null, [Wed Dec 31 
18:00:00 CST 1969]]]]> but was:<...alse, true], [null, [Thu Jan 01 01:00:00 CET 
1970]]]]>

Original comment by davidkar...@gmail.com on 15 Nov 2010 at 9:47

[GoogleCodeExporter]

I fixed the Date based unit test, it should no longer be time-zone dependent.

Original comment by eric.dalquist on 20 Jan 2011 at 2:26

• Changed state: Fixed

[GoogleCodeExporter]

Original comment by eric.dalquist on 9 Mar 2011 at 5:24

• Added labels: Milestone-Release1.1.3

[GoogleCodeExporter]

Is the relased pushed out to the maven repo yet?

Original comment by davidkar...@gmail.com on 9 Mar 2011 at 5:51

[GoogleCodeExporter]

We push releases to oss.sonatype.org when we create them:
https://oss.sonatype.org/content/repositories/releases/com/googlecode/ehcache-sp
ring-annotations/

They sync to the central repo periodically (hourly I think) so it should be 
there soon.

Original comment by eric.dalquist on 9 Mar 2011 at 5:53