[jira] [Commented] (CAY-1872) Easier mapping of default listener methods

["Andrus Adamchik (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CAY-1872?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13967469#comment-13967469 ] 

Andrus Adamchik commented on CAY-1872:
--------------------------------------

I see a few problems with the patch:

1. Reflection logic in CallbackOnListener.performCallback .. This is a very performance-sensitive code. We shouldn't be doing any refelection here. All the information that we need should be precompiled during callback registration.

2. LifecycleCallbackEventHandler. The new method is called "addDefaultListener". In callbacks "default" means "match any entity". What we are trying to do here is to actually match a specific entity (or an inheritance subtree of entities). Perhaps we should get rid of the "default" concept completely, and just treat all callbacks as matching a certain set of entities.

3. LifecycleCallbackRegistry - the check for parameter array length is not taking into account the rule about listener methods - they must have one and only one parameter: http://cayenne.apache.org/docs/3.1/cayenne-guide/lifecycle-events.html#callback-non-persistent  In fact Cayenne will throw an exception downstream if this is not the case (see CallbackOnListener.verifyMethod)

So need to discuss this further.

> Easier mapping of default listener methods
> ------------------------------------------
>
>                 Key: CAY-1872
>                 URL: https://issues.apache.org/jira/browse/CAY-1872
>             Project: Cayenne
>          Issue Type: Improvement
>            Reporter: Andrus Adamchik
>            Assignee: Andrus Adamchik
>         Attachments: cay1872.patch, cay1872_test.patch, cay1872v2.patch, cay1872v2_test.patch
>
>
> IIRC a listener method like this doesn't match anything (the actual annotation can be any lifecycle annotation.. the important part is absent entity filter) :
> @PrePersist
> public void doSomething(T x) { }
> We should actually match that based on the "T" type instead. So if T is an Object or DataObject, it should match every entity in the model, if T is a specific entity class, it should match that specific class. So more generally it should match entities that are of type T or any subtype of T
> Performance is a consideration... T can be a class or an interface. I can see how we can make this efficient with T being a class. With T being an interface other than DataObject or Persistent, we'll need to analyze all persistent classes in the Model, which can affect performance. I guess we can make a partial implementation initially supporting T as a class and DataObject / Persistent interfaces, and then see how bad is a class hierarchy scan with an arbitrary interface. 



--
This message was sent by Atlassian JIRA
(v6.2#6252)