{"action":"created","comment":{"url":"https://api.github.com/repos/playframework/playframework/pulls/comments/64072054","id":64072054,"diff_hunk":"@@ -78,4 +79,27 @@ object ConnectionPool {\n     }\n \n   }\n+\n+  /**\n+   * Wraps a data source in a org.jdbcdslog.LogSqlDataSource if the logSql configuration property is set to true.\n+   */\n+  def maybeWrapDataSource(dataSource: DataSource, configuration: Config): DataSource = {","path":"framework/src/play-jdbc/src/main/scala/play/api/db/ConnectionPool.scala","position":16,"original_position":16,"commit_id":"6d920e1a3bd747b2fbceda7e84ef2609ffef206b","original_commit_id":"eec64ee689853cf3adb3fc2eba50865cc78adb74","user":{"login":"marcospereira","id":4576,"avatar_url":"https://avatars.githubusercontent.com/u/4576?v=3","gravatar_id":"","url":"https://api.github.com/users/marcospereira","html_url":"https://github.com/marcospereira","followers_url":"https://api.github.com/users/marcospereira/followers","following_url":"https://api.github.com/users/marcospereira/following{/other_user}","gists_url":"https://api.github.com/users/marcospereira/gists{/gist_id}","starred_url":"https://api.github.com/users/marcospereira/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/marcospereira/subscriptions","organizations_url":"https://api.github.com/users/marcospereira/orgs","repos_url":"https://api.github.com/users/marcospereira/repos","events_url":"https://api.github.com/users/marcospereira/events{/privacy}","received_events_url":"https://api.github.com/users/marcospereira/received_events","type":"User","site_admin":false},"body":"I'm ok with you passing the configuration instead of a boolean. This is just a small detail. Also, I would go with the if/else instead of the match/case.","created_at":"2016-05-20T16:49:07Z","updated_at":"2016-05-20T16:49:07Z","html_url":"https://github.com/playframework/playframework/pull/6150#discussion_r64072054","pull_request_url":"https://api.github.com/repos/playframework/playframework/pulls/6150","_links":{"self":{"href":"https://api.github.com/repos/playframework/playframework/pulls/comments/64072054"},"html":{"href":"https://github.com/playframework/playframework/pull/6150#discussion_r64072054"},"pull_request":{"href":"https://api.github.com/repos/playframework/playframework/pulls/6150"}}},"pull_request":{"url":"https://api.github.com/repos/playframework/playframework/pulls/6150","id":69692114,"html_url":"https://github.com/playframework/playframework/pull/6150","diff_url":"https://github.com/playframework/playframework/pull/6150.diff","patch_url":"https://github.com/playframework/playframework/pull/6150.patch","issue_url":"https://api.github.com/repos/playframework/playframework/issues/6150","number":6150,"state":"open","locked":false,"title":"jdbcdslog is now enabled at the connection pool / data source level","user":{"login":"sevasm","id":366521,"avatar_url":"https://avatars.githubusercontent.com/u/366521?v=3","gravatar_id":"","url":"https://api.github.com/users/sevasm","html_url":"https://github.com/sevasm","followers_url":"https://api.github.com/users/sevasm/followers","following_url":"https://api.github.com/users/sevasm/following{/other_user}","gists_url":"https://api.github.com/users/sevasm/gists{/gist_id}","starred_url":"https://api.github.com/users/sevasm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sevasm/subscriptions","organizations_url":"https://api.github.com/users/sevasm/orgs","repos_url":"https://api.github.com/users/sevasm/repos","events_url":"https://api.github.com/users/sevasm/events{/privacy}","received_events_url":"https://api.github.com/users/sevasm/received_events","type":"User","site_admin":false},"body":"# Pull Request Checklist\r\n\r\n* [x] Have you read [How to write the perfect pull request](https://github.com/blog/1943-how-to-write-the-perfect-pull-request)?\r\n* [x] Have you read through the [contributor guidelines](https://www.playframework.com/contributing)?\r\n* [x] Have you signed the [Lightbend CLA](https://www.lightbend.com/contribute/cla)?\r\n* [x] Have you [squashed your commits](https://www.playframework.com/documentation/2.4.x/WorkingWithGit#Squashing-commits)?\r\n* [x] Have you added copyright headers to new files?\r\n* [x] Have you checked that both Scala and Java APIs are updated?\r\n* [x] Have you updated the documentation for both Scala and Java sections?\r\n* [x] Have you added tests for any changed functionality?\r\n\r\n## Fixes\r\n\r\nFixes #6149 \r\n\r\n## Purpose\r\n\r\nTo enable SQL logging with jdbcdslog the data source needs to be wrapped in a org.jdbcdslog.LogSqlDataSource. This was previously done at the database API level (in Databases.scala) before data sources were registered with JNDI. This causes database operations that use JPA to not be logged as their data source is not wrapped in a org.jdbcdslog.LogSqlDataSource. Now each supported connection pool wraps its own data sources when they are created and unwraps them before they are closed.\r\n\r\n## Background Context\r\n\r\nThe data sources are already registered with JNDI in the implementation classes for both supported connection pools, so I modified both to also automatically wrap the data source in a org.jdbcdslog.LogSqlDataSource if logSql is set to true. I have also moved the code that unwraps a data source when it needs to be closed. Since the functionality is the same for both connection pool implementations I placed the two methods (wrap data source and unwrap data source) in the ConnectionPool object.\r\n\r\nIt seems that registering any new data sources with JNDI is left up to the implementation of each connection pool at the moment, so it seemed appropriate that wrapping and unwrapping their data sources to support logging would be done there as well. Although I am not a huge fan of this as it makes implementing a new connection pool more difficult (have to remember to bind its data sources to JNDI and to wrap / unwrap the data sources to enable logging).\r\n\r\nThe code that actually wraps / unwraps the data sources could be copied into both implementations directly and removed from ConnectionPool to match what is done with JNDI registration code at the moment if that is preferable.\r\n\r\nApologies for not writing any test cases for this, not sure how I would go about that unfortunately. Maybe someone can direct me? But I can confirm I have tested the changes with both Java and Scala versions of Play.\r\n\r\nEdit: I have now added a few tests to check whether the logSql configuration property is applied correctly and whether the right data source class is used both using the Database API and the JPA API.\r\n\r\n## References\r\n\r\nHere's the original post about this issue on the Google group:\r\n\r\nhttps://groups.google.com/d/msgid/play-framework/bd37c292-60b7-457c-a01c-018e824c8c70%40googlegroups.com?utm_medium=email&utm_source=footer\r\n","created_at":"2016-05-11T15:27:06Z","updated_at":"2016-05-20T16:49:07Z","closed_at":null,"merged_at":null,"merge_commit_sha":"4119507845d04271fa970f77159f404ff23e5367","assignee":null,"milestone":{"url":"https://api.github.com/repos/playframework/playframework/milestones/40","html_url":"https://github.com/playframework/playframework/milestones/2.5.4","labels_url":"https://api.github.com/repos/playframework/playframework/milestones/40/labels","id":1739401,"number":40,"title":"2.5.4","description":"","creator":{"login":"marcospereira","id":4576,"avatar_url":"https://avatars.githubusercontent.com/u/4576?v=3","gravatar_id":"","url":"https://api.github.com/users/marcospereira","html_url":"https://github.com/marcospereira","followers_url":"https://api.github.com/users/marcospereira/followers","following_url":"https://api.github.com/users/marcospereira/following{/other_user}","gists_url":"https://api.github.com/users/marcospereira/gists{/gist_id}","starred_url":"https://api.github.com/users/marcospereira/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/marcospereira/subscriptions","organizations_url":"https://api.github.com/users/marcospereira/orgs","repos_url":"https://api.github.com/users/marcospereira/repos","events_url":"https://api.github.com/users/marcospereira/events{/privacy}","received_events_url":"https://api.github.com/users/marcospereira/received_events","type":"User","site_admin":false},"open_issues":8,"closed_issues":9,"state":"open","created_at":"2016-04-29T22:08:28Z","updated_at":"2016-05-20T13:30:13Z","due_on":null,"closed_at":null},"commits_url":"https://api.github.com/repos/playframework/playframework/pulls/6150/commits","review_comments_url":"https://api.github.com/repos/playframework/playframework/pulls/6150/comments","review_comment_url":"https://api.github.com/repos/playframework/playframework/pulls/comments{/number}","comments_url":"https://api.github.com/repos/playframework/playframework/issues/6150/comments","statuses_url":"https://api.github.com/repos/playframework/playframework/statuses/6d920e1a3bd747b2fbceda7e84ef2609ffef206b","head":{"label":"sevasm:issue-6149","ref":"issue-6149","sha":"6d920e1a3bd747b2fbceda7e84ef2609ffef206b","user":{"login":"sevasm","id":366521,"avatar_url":"https://avatars.githubusercontent.com/u/366521?v=3","gravatar_id":"","url":"https://api.github.com/users/sevasm","html_url":"https://github.com/sevasm","followers_url":"https://api.github.com/users/sevasm/followers","following_url":"https://api.github.com/users/sevasm/following{/other_user}","gists_url":"https://api.github.com/users/sevasm/gists{/gist_id}","starred_url":"https://api.github.com/users/sevasm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sevasm/subscriptions","organizations_url":"https://api.github.com/users/sevasm/orgs","repos_url":"https://api.github.com/users/sevasm/repos","events_url":"https://api.github.com/users/sevasm/events{/privacy}","received_events_url":"https://api.github.com/users/sevasm/received_events","type":"User","site_admin":false},"repo":{"id":58554195,"name":"playframework-6149","full_name":"sevasm/playframework-6149","owner":{"login":"sevasm","id":366521,"avatar_url":"https://avatars.githubusercontent.com/u/366521?v=3","gravatar_id":"","url":"https://api.github.com/users/sevasm","html_url":"https://github.com/sevasm","followers_url":"https://api.github.com/users/sevasm/followers","following_url":"https://api.github.com/users/sevasm/following{/other_user}","gists_url":"https://api.github.com/users/sevasm/gists{/gist_id}","starred_url":"https://api.github.com/users/sevasm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sevasm/subscriptions","organizations_url":"https://api.github.com/users/sevasm/orgs","repos_url":"https://api.github.com/users/sevasm/repos","events_url":"https://api.github.com/users/sevasm/events{/privacy}","received_events_url":"https://api.github.com/users/sevasm/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/sevasm/playframework-6149","description":"Play Framework","fork":true,"url":"https://api.github.com/repos/sevasm/playframework-6149","forks_url":"https://api.github.com/repos/sevasm/playframework-6149/forks","keys_url":"https://api.github.com/repos/sevasm/playframework-6149/keys{/key_id}","collaborators_url":"https://api.github.com/repos/sevasm/playframework-6149/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/sevasm/playframework-6149/teams","hooks_url":"https://api.github.com/repos/sevasm/playframework-6149/hooks","issue_events_url":"https://api.github.com/repos/sevasm/playframework-6149/issues/events{/number}","events_url":"https://api.github.com/repos/sevasm/playframework-6149/events","assignees_url":"https://api.github.com/repos/sevasm/playframework-6149/assignees{/user}","branches_url":"https://api.github.com/repos/sevasm/playframework-6149/branches{/branch}","tags_url":"https://api.github.com/repos/sevasm/playframework-6149/tags","blobs_url":"https://api.github.com/repos/sevasm/playframework-6149/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/sevasm/playframework-6149/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/sevasm/playframework-6149/git/refs{/sha}","trees_url":"https://api.github.com/repos/sevasm/playframework-6149/git/trees{/sha}","statuses_url":"https://api.github.com/repos/sevasm/playframework-6149/statuses/{sha}","languages_url":"https://api.github.com/repos/sevasm/playframework-6149/languages","stargazers_url":"https://api.github.com/repos/sevasm/playframework-6149/stargazers","contributors_url":"https://api.github.com/repos/sevasm/playframework-6149/contributors","subscribers_url":"https://api.github.com/repos/sevasm/playframework-6149/subscribers","subscription_url":"https://api.github.com/repos/sevasm/playframework-6149/subscription","commits_url":"https://api.github.com/repos/sevasm/playframework-6149/commits{/sha}","git_commits_url":"https://api.github.com/repos/sevasm/playframework-6149/git/commits{/sha}","comments_url":"https://api.github.com/repos/sevasm/playframework-6149/comments{/number}","issue_comment_url":"https://api.github.com/repos/sevasm/playframework-6149/issues/comments{/number}","contents_url":"https://api.github.com/repos/sevasm/playframework-6149/contents/{+path}","compare_url":"https://api.github.com/repos/sevasm/playframework-6149/compare/{base}...{head}","merges_url":"https://api.github.com/repos/sevasm/playframework-6149/merges","archive_url":"https://api.github.com/repos/sevasm/playframework-6149/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/sevasm/playframework-6149/downloads","issues_url":"https://api.github.com/repos/sevasm/playframework-6149/issues{/number}","pulls_url":"https://api.github.com/repos/sevasm/playframework-6149/pulls{/number}","milestones_url":"https://api.github.com/repos/sevasm/playframework-6149/milestones{/number}","notifications_url":"https://api.github.com/repos/sevasm/playframework-6149/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/sevasm/playframework-6149/labels{/name}","releases_url":"https://api.github.com/repos/sevasm/playframework-6149/releases{/id}","deployments_url":"https://api.github.com/repos/sevasm/playframework-6149/deployments","created_at":"2016-05-11T14:53:46Z","updated_at":"2016-05-11T14:54:24Z","pushed_at":"2016-05-13T08:05:17Z","git_url":"git://github.com/sevasm/playframework-6149.git","ssh_url":"git@github.com:sevasm/playframework-6149.git","clone_url":"https://github.com/sevasm/playframework-6149.git","svn_url":"https://github.com/sevasm/playframework-6149","homepage":"http://www.playframework.com/","size":84804,"stargazers_count":0,"watchers_count":0,"language":"Scala","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"playframework:master","ref":"master","sha":"cfbda4ce9d4d773ab0d400d932271adfbb1e892c","user":{"login":"playframework","id":319107,"avatar_url":"https://avatars.githubusercontent.com/u/319107?v=3","gravatar_id":"","url":"https://api.github.com/users/playframework","html_url":"https://github.com/playframework","followers_url":"https://api.github.com/users/playframework/followers","following_url":"https://api.github.com/users/playframework/following{/other_user}","gists_url":"https://api.github.com/users/playframework/gists{/gist_id}","starred_url":"https://api.github.com/users/playframework/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/playframework/subscriptions","organizations_url":"https://api.github.com/users/playframework/orgs","repos_url":"https://api.github.com/users/playframework/repos","events_url":"https://api.github.com/users/playframework/events{/privacy}","received_events_url":"https://api.github.com/users/playframework/received_events","type":"Organization","site_admin":false},"repo":{"id":2340549,"name":"playframework","full_name":"playframework/playframework","owner":{"login":"playframework","id":319107,"avatar_url":"https://avatars.githubusercontent.com/u/319107?v=3","gravatar_id":"","url":"https://api.github.com/users/playframework","html_url":"https://github.com/playframework","followers_url":"https://api.github.com/users/playframework/followers","following_url":"https://api.github.com/users/playframework/following{/other_user}","gists_url":"https://api.github.com/users/playframework/gists{/gist_id}","starred_url":"https://api.github.com/users/playframework/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/playframework/subscriptions","organizations_url":"https://api.github.com/users/playframework/orgs","repos_url":"https://api.github.com/users/playframework/repos","events_url":"https://api.github.com/users/playframework/events{/privacy}","received_events_url":"https://api.github.com/users/playframework/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/playframework/playframework","description":"Play Framework","fork":false,"url":"https://api.github.com/repos/playframework/playframework","forks_url":"https://api.github.com/repos/playframework/playframework/forks","keys_url":"https://api.github.com/repos/playframework/playframework/keys{/key_id}","collaborators_url":"https://api.github.com/repos/playframework/playframework/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/playframework/playframework/teams","hooks_url":"https://api.github.com/repos/playframework/playframework/hooks","issue_events_url":"https://api.github.com/repos/playframework/playframework/issues/events{/number}","events_url":"https://api.github.com/repos/playframework/playframework/events","assignees_url":"https://api.github.com/repos/playframework/playframework/assignees{/user}","branches_url":"https://api.github.com/repos/playframework/playframework/branches{/branch}","tags_url":"https://api.github.com/repos/playframework/playframework/tags","blobs_url":"https://api.github.com/repos/playframework/playframework/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/playframework/playframework/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/playframework/playframework/git/refs{/sha}","trees_url":"https://api.github.com/repos/playframework/playframework/git/trees{/sha}","statuses_url":"https://api.github.com/repos/playframework/playframework/statuses/{sha}","languages_url":"https://api.github.com/repos/playframework/playframework/languages","stargazers_url":"https://api.github.com/repos/playframework/playframework/stargazers","contributors_url":"https://api.github.com/repos/playframework/playframework/contributors","subscribers_url":"https://api.github.com/repos/playframework/playframework/subscribers","subscription_url":"https://api.github.com/repos/playframework/playframework/subscription","commits_url":"https://api.github.com/repos/playframework/playframework/commits{/sha}","git_commits_url":"https://api.github.com/repos/playframework/playframework/git/commits{/sha}","comments_url":"https://api.github.com/repos/playframework/playframework/comments{/number}","issue_comment_url":"https://api.github.com/repos/playframework/playframework/issues/comments{/number}","contents_url":"https://api.github.com/repos/playframework/playframework/contents/{+path}","compare_url":"https://api.github.com/repos/playframework/playframework/compare/{base}...{head}","merges_url":"https://api.github.com/repos/playframework/playframework/merges","archive_url":"https://api.github.com/repos/playframework/playframework/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/playframework/playframework/downloads","issues_url":"https://api.github.com/repos/playframework/playframework/issues{/number}","pulls_url":"https://api.github.com/repos/playframework/playframework/pulls{/number}","milestones_url":"https://api.github.com/repos/playframework/playframework/milestones{/number}","notifications_url":"https://api.github.com/repos/playframework/playframework/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/playframework/playframework/labels{/name}","releases_url":"https://api.github.com/repos/playframework/playframework/releases{/id}","deployments_url":"https://api.github.com/repos/playframework/playframework/deployments","created_at":"2011-09-07T09:24:08Z","updated_at":"2016-05-20T16:14:13Z","pushed_at":"2016-05-20T03:26:20Z","git_url":"git://github.com/playframework/playframework.git","ssh_url":"git@github.com:playframework/playframework.git","clone_url":"https://github.com/playframework/playframework.git","svn_url":"https://github.com/playframework/playframework","homepage":"http://www.playframework.com/","size":96230,"stargazers_count":7879,"watchers_count":7879,"language":"Scala","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":2735,"mirror_url":null,"open_issues_count":269,"forks":2735,"open_issues":269,"watchers":7879,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/playframework/playframework/pulls/6150"},"html":{"href":"https://github.com/playframework/playframework/pull/6150"},"issue":{"href":"https://api.github.com/repos/playframework/playframework/issues/6150"},"comments":{"href":"https://api.github.com/repos/playframework/playframework/issues/6150/comments"},"review_comments":{"href":"https://api.github.com/repos/playframework/playframework/pulls/6150/comments"},"review_comment":{"href":"https://api.github.com/repos/playframework/playframework/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/playframework/playframework/pulls/6150/commits"},"statuses":{"href":"https://api.github.com/repos/playframework/playframework/statuses/6d920e1a3bd747b2fbceda7e84ef2609ffef206b"}}}}
{"actor":{"display_login":"marcospereira"}}