• New Feature
  • Status: Closed
  • 1 Critical
  • Resolution: Fixed
  • teck
  • Reporter: steve
  • April 08, 2011
  • 0
  • Watchers: 3
  • July 27, 2012
  • April 25, 2011

Description

There are cases where people want to write generic Extractors and then use them with multiple attributes. The problem is that the Extractor doesn’t know what attribute he is associated with. My thinking is that we create a sub interface that has the method

attributeFor(Element element, Attribute name)

Open to other solutions as well.

Comments

Tim Eck 2011-04-08

I agree we need to provide some relieve on making extractors easier. The model we have now is just a touch too simplistic.

A sub-interface (I assume you mean a subclass of the existing one) seems like a reasonable thing to maintain API compatibility. If we want to me more lax on the compatibility thing we just update the existing method signature or a new method to the existing interface.

Another possibility is to introduce a factory concept for creating extractor instances. Another thing would be to allow extractor instances to be directly handed to the cache but that would only help people doing programatic cache config

gluck 2011-04-11

Maybe I am thick but I do not see how tying an extractor to a attribute makes it generic.

Right now we have:

Are we saying that we want one class that can handle both of the above examples? So:

If so then we do need to change the interface.

We have:

//use this then the extractor is only used for one attribute Object attributeFor(Element element) throws AttributeExtractorException;

So we need to add (as Steve suggests):

//use this when the attribute is used for multiple attributes Object attributeFor(Element element, Attribute attribute) throws AttributeExtractorException;

Backward compatibility in the SPI matters less than in the API. And it matters less when we have a new feature that very few people has used. We could sub-interface but that just complicates things for everyone else as the new one is the one they will want to use most.

So I suggest we add it directly to the interface.

As to Tim’s other ideas the usual way it works is that we use an AbstractFactory pattern so that someone creates a concrete factory and then gives back an object that meets an interface. From what I can tell it is the interface rather than how the implementation gets created that is the concern here.

Tim Eck 2011-04-11

I think the basic issue is that people want to write a single extractor class that can effectively switch on the attribute instead of writing a separate class for each.

Actually I’m a little confused now about having two methods. How do we decide to call one or the other? Seems maybe like if we’re okay with not being compatible that we should just add the parameter to the existing method?

A factory might be useful when/if people want these extractors to be stateful in any way. I don’t have a argument for that so it would be best as you suggest to tweak interfaces here for now

gluck 2011-04-11

Yep just changing the signature is probably the best way to go.

Tim Eck 2011-04-12

Final question is where is it okay to introduce an API break? Specifically is it 2.5.x only or is it okay in 2.4.x?

This JIRA is currently in 3.5.x target which to me is kinda ambiguous when it comes to the applicable ehcache-core version(s)

Tim Eck 2011-04-12

This change will affect code in ehcache-core and tim-ehcache if that helps make the decision about when to do it.

Tim Eck 2011-04-12

Another question: It would be far easier in the existing code to make the method this:

Object attributeFor(Element element, String attributeName)

That is passing String attributeName instead of an Attribute instance.

Any thoughts?

Tim Eck 2011-04-12

Perhaps we’ll have to meet/chat about this….

To avoid an interface change we could maybe try moving more towards the direction of providing state to the extractors at construction time. Not necessarily a full blown factory but perhaps a way to specify constructor parameters in the cache config. There could be a standard parameters (such as the attribute) in the mix here.

gluck 2011-04-12

Yeah I was thinking about String. The attributes are always Strings. The extra stuff in the Attribute class is not required for the extraction.

Tim Eck 2011-04-12

Okay, new string attributeName parameter to the existing method, good deal.

can this be in 2.4.x?

Tim Eck 2011-04-25

this API break is now in 2.4.x line for 2.4.3 core release

Steve Harris 2011-04-25

Did you update the apt file as well?

Tim Eck 2011-04-25

the apt doesn’t reference the exact method signature