On Tue, Sep 23, 2014 at 9:39 AM, William Burns <mudokon...@gmail.com> wrote: > On Tue, Sep 23, 2014 at 9:31 AM, Mircea Markus <mmar...@redhat.com> wrote: >> >> On Sep 23, 2014, at 16:27, Emmanuel Bernard <emman...@hibernate.org> wrote: >> >>> >>> On 23 Sep 2014, at 14:53, Mircea Markus <mmar...@redhat.com> wrote: >>> >>>> On Sep 23, 2014, at 15:18, Emmanuel Bernard <emman...@hibernate.org> wrote: >>>> >>>>>> I am not sold on this as it seems pretty trivial to decipher which >>>>>> operation is which and the information would be present on the >>>>>> javadocs as well. >>>>> >>>>> I very strongly disagree. Cf the other thread with Radim 's comment on >>>>> topology error. >>>>> And think about *future* evolutions. The enum would make that much safer. >>>>> In the bin enum world you would have to introduce a new >>>>> YetAnotherKeyValueFilter interface :) >>>> >>>> Nicer than an enum would be an explicit method, e.g. >>>> handlePut/handleDelete/handleCreate/handleUpdate, as these would also >>>> receive the appropriate param list. Of course this means moving away from >>>> the KeyValueFilter to an UpdateFilter (good name, Radim) used only for >>>> cluster listeners. > > I like the name as well :) The only thing that I dislike about the > extra methods is the fact that it isn't a Functional interface, which > would be nice to have when we ever move to Java 8, but that may be > thinking too far into the future :P > >>>> Will, what would be the overall impact on the A > > The biggest part is the usage with the cluster iterator. Currently > the Listener uses the same filter that it is provided to also do the > iteration. If we want to go down the line of having the extra > interface(s), which overall I do like, then I am thinking we may want > to change the Listener annotation to no longer have an > includeCurrentState parameter and instead add a new method to the > addListener method of Cache that takes a KeyValueFilter and the new > UpdateFilter (as well as the 2 converters). I can then add in 2 > bridge implementations so that you don't have to implement the other > if your implementation can handle both types. Also from the other > post it seems that I should add the retry boolean to all the > appropriate methods so that you can have a chance to detect if an > update was missed. Unless this seems to cumbersome? > >>> >>> If you do that you must also provide an abstract class with default noop >>> operations that filter implementations would extend. Otherwise you are back >>> with backward compatibility problems. >> >> KeyValueFilter was introduced in 7.0, or other backward compatibility >> problem you have in mind? > > I believe Emmanuel is referring to if we added additional operations > to the filter, but I am not sure what other operations we would want > to add to it. If anything we would probably make a different type of > filter specific to its use case.
Reread the other email again and actually it could be used to show different permutations like the retry case (eq RETRIED_CREATE), but it seems like the code in that one method would get pretty complex pretty fast having to handle all the various cases. > >> >> Cheers, >> -- >> Mircea Markus >> Infinispan lead (www.infinispan.org) >> >> >> >> >> >> _______________________________________________ >> infinispan-dev mailing list >> infinispan-dev@lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/infinispan-dev _______________________________________________ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev