[jira] Commented: (CAY-1486) Switch QueryLogger to a DI-managed object

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

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

Andrus Adamchik commented on CAY-1486:
--------------------------------------

Andrei, I think this is moving in the right direction. I can suggest a few cosmetic improvements:

* Since we want deprecated QueryLogger to still work as before, I suggest maybe initializing its static "logger" var inside a static block in QueryLogger itself. In this case we won't need a special bootstrap code in CayenneCase, ServerCase, etc.  Setting it in ServerModule still looks appropriate (even if redundant) IMO

* Declaring JdbcEventLogger as a "private" ivar inside DataDomain and DefaultDbAdapterFactory leaves it fully dependent on injection. This is not necessarily wrong, but I guess following a pattern of other stack classes, I suggest making it protected to allow potential subclasses to access it (won't go as far as creating getters/setters just yet)...

* One last thing... I think if we also replace QueryFormatter -related logic with a subclass of CommonsJdbcEventLogger (and remove the config property enabling it), the patch will be ready to be committed, even without support for the full injection into the lower layers of the framework

> Switch QueryLogger to a DI-managed object
> -----------------------------------------
>
>                 Key: CAY-1486
>                 URL: https://issues.apache.org/jira/browse/CAY-1486
>             Project: Cayenne
>          Issue Type: Task
>          Components: Core Library
>    Affects Versions: 3.1M1
>            Reporter: Andrus Adamchik
>            Assignee: Andrus Adamchik
>             Fix For: 3.1M1
>
>         Attachments: CAY-1486.patch
>
>
> Will replace static QueryLogger with a DI-managed service. Preliminary name of the service interface is org.apache.cayenne.log.JdbcEventLogger, implementation - org.apache.cayenne.log.CommonsJdbcEventLogger (meaning it is based on commons-logging). "Event" part of the name comes from this: CAY-1173. The API of JdbcEventLogger will initially match QueryLogger (only it won't be static). The original QueryLogger should be deprecated. 
> This task will require injection into lower layers of Cayenne runtime below DataDomain. Not yet sure how we should approach it. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.