[infinispan-dev] Feedback and requests on clustered and remote listeners

[William Burns]

On Tue, Sep 23, 2014 at 9:39 AM, William Burns <mudokonman at gmail.com> wrote:
> On Tue, Sep 23, 2014 at 9:31 AM, Mircea Markus <mmarkus at redhat.com> wrote:
>>
>> On Sep 23, 2014, at 16:27, Emmanuel Bernard <emmanuel at hibernate.org> wrote:
>>
>>>
>>> On 23 Sep 2014, at 14:53, Mircea Markus <mmarkus at redhat.com> wrote:
>>>
>>>> On Sep 23, 2014, at 15:18, Emmanuel Bernard <emmanuel at 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

Actually while working and thinking on this it seems it may be easiest
to exclude the usage of KeyValueFilter in the listener pieces
completely and instead leave the annotation as it is now.  Instead the
provided CacheEventFilter would be wrapped by a KeyValueFilter
implement that just called the new method as if it was a create event
for each value while iterating on them.  I am thinking this is the
cleanest.  Do you guys have any opinions?  It would also keep intact a
lot of existing code and APIs.

> 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.
>
>>
>> Cheers,
>> --
>> Mircea Markus
>> Infinispan lead (www.infinispan.org)
>>
>>
>>
>>
>>
>> _______________________________________________
>> infinispan-dev mailing list
>> infinispan-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev