[jira] [Commented] (DBUTILS-124) Introduce SPI to add more column, property handlers

2018-04-20 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/DBUTILS-124?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16446246#comment-16446246
 ] 

ASF GitHub Bot commented on DBUTILS-124:


Github user hdevalke closed the pull request at:

https://github.com/apache/commons-dbutils/pull/3


> Introduce SPI to add more column, property handlers
> ---
>
> Key: DBUTILS-124
> URL: https://issues.apache.org/jira/browse/DBUTILS-124
> Project: Commons DbUtils
>  Issue Type: New Feature
>Reporter: Carl Hall
>Assignee: Carl Hall
>Priority: Major
> Fix For: 1.7
>
>
> The column types and property types handled by {{BeanProcessor}} are hard 
> coded to the processor.  We already use a common return type, so we could add 
> a services approach using the spi built into the jdk.  This should also allow 
> other types to be handled outside of {{commons-dbutils}}.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (DBUTILS-124) Introduce SPI to add more column, property handlers

2018-01-16 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/DBUTILS-124?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16326934#comment-16326934
 ] 

ASF GitHub Bot commented on DBUTILS-124:


Github user objectboy2016 commented on the issue:

https://github.com/apache/commons-dbutils/pull/3
  
**Why not design a named parameter** 
such as: 
 string sql="select * from user where name=:name";
 ...
 runquery.addParameter("name","paul");

So simple and straightforward
No problem with the number, but the parameters of many will be a disaster

such as:
insert into a (a, b, c, d, f, g, i, e, g, k) values (?,?,?,?,?,?,?,?,?)

My eyes have been spent


> Introduce SPI to add more column, property handlers
> ---
>
> Key: DBUTILS-124
> URL: https://issues.apache.org/jira/browse/DBUTILS-124
> Project: Commons DbUtils
>  Issue Type: New Feature
>Reporter: Carl Hall
>Assignee: Carl Hall
>Priority: Major
> Fix For: 1.7
>
>
> The column types and property types handled by {{BeanProcessor}} are hard 
> coded to the processor.  We already use a common return type, so we could add 
> a services approach using the spi built into the jdk.  This should also allow 
> other types to be handled outside of {{commons-dbutils}}.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (DBUTILS-124) Introduce SPI to add more column, property handlers

2017-10-10 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/DBUTILS-124?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16199512#comment-16199512
 ] 

ASF GitHub Bot commented on DBUTILS-124:


Github user garydgregory commented on the issue:

https://github.com/apache/commons-dbutils/pull/3
  
I created https://issues.apache.org/jira/browse/DBUTILS-135 to track this 
fix. Committed to git master.


> Introduce SPI to add more column, property handlers
> ---
>
> Key: DBUTILS-124
> URL: https://issues.apache.org/jira/browse/DBUTILS-124
> Project: Commons DbUtils
>  Issue Type: New Feature
>Reporter: Carl Hall
>Assignee: Carl Hall
> Fix For: 1.7
>
>
> The column types and property types handled by {{BeanProcessor}} are hard 
> coded to the processor.  We already use a common return type, so we could add 
> a services approach using the spi built into the jdk.  This should also allow 
> other types to be handled outside of {{commons-dbutils}}.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (DBUTILS-124) Introduce SPI to add more column, property handlers

2017-10-10 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/DBUTILS-124?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16199258#comment-16199258
 ] 

ASF GitHub Bot commented on DBUTILS-124:


Github user hdevalke commented on a diff in the pull request:

https://github.com/apache/commons-dbutils/pull/3#discussion_r143833029
  
--- Diff: src/main/java/org/apache/commons/dbutils/BeanProcessor.java ---
@@ -65,19 +65,21 @@
  */
 private static final Map primitiveDefaults = new 
HashMap();
 
-/**
- * ServiceLoader to find ColumnHandler implementations on 
the classpath.  The iterator for this is
- * lazy and each time iterator() is called.
- */
-// FIXME: I think this instantiates new handlers on each iterator() 
call. This might be worth caching upfront.
-private static final ServiceLoader columnHandlers = 
ServiceLoader.load(ColumnHandler.class);
+private static final List columnHandlers = new 
ArrayList();
 
-/**
- * ServiceLoader to find PropertyHandler implementations 
on the classpath.  The iterator for this is
- * lazy and each time iterator() is called.
- */
-// FIXME: I think this instantiates new handlers on each iterator() 
call. This might be worth caching upfront.
-private static final ServiceLoader propertyHandlers = 
ServiceLoader.load(PropertyHandler.class);
+static{
+for (ColumnHandler h : ServiceLoader.load(ColumnHandler.class)) {
+columnHandlers.add(h);
+}
+}
+
+private static final List propertyHandlers = new 
ArrayList();
+
+static {
+for (PropertyHandler h : 
ServiceLoader.load(PropertyHandler.class)) {
+propertyHandlers.add(h);
--- End diff --

I didn't see there was already a static block initializing 
`primitiveDefaults`. I find it more readable initializing each static variable 
in their own block. I did an update and put all in one static block.


> Introduce SPI to add more column, property handlers
> ---
>
> Key: DBUTILS-124
> URL: https://issues.apache.org/jira/browse/DBUTILS-124
> Project: Commons DbUtils
>  Issue Type: New Feature
>Reporter: Carl Hall
>Assignee: Carl Hall
> Fix For: 1.7
>
>
> The column types and property types handled by {{BeanProcessor}} are hard 
> coded to the processor.  We already use a common return type, so we could add 
> a services approach using the spi built into the jdk.  This should also allow 
> other types to be handled outside of {{commons-dbutils}}.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (DBUTILS-124) Introduce SPI to add more column, property handlers

2017-10-10 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/DBUTILS-124?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16199193#comment-16199193
 ] 

ASF GitHub Bot commented on DBUTILS-124:


Github user garydgregory commented on a diff in the pull request:

https://github.com/apache/commons-dbutils/pull/3#discussion_r143822953
  
--- Diff: src/main/java/org/apache/commons/dbutils/BeanProcessor.java ---
@@ -65,19 +65,21 @@
  */
 private static final Map primitiveDefaults = new 
HashMap();
 
-/**
- * ServiceLoader to find ColumnHandler implementations on 
the classpath.  The iterator for this is
- * lazy and each time iterator() is called.
- */
-// FIXME: I think this instantiates new handlers on each iterator() 
call. This might be worth caching upfront.
-private static final ServiceLoader columnHandlers = 
ServiceLoader.load(ColumnHandler.class);
+private static final List columnHandlers = new 
ArrayList();
 
-/**
- * ServiceLoader to find PropertyHandler implementations 
on the classpath.  The iterator for this is
- * lazy and each time iterator() is called.
- */
-// FIXME: I think this instantiates new handlers on each iterator() 
call. This might be worth caching upfront.
-private static final ServiceLoader propertyHandlers = 
ServiceLoader.load(PropertyHandler.class);
+static{
+for (ColumnHandler h : ServiceLoader.load(ColumnHandler.class)) {
+columnHandlers.add(h);
+}
+}
+
+private static final List propertyHandlers = new 
ArrayList();
+
+static {
+for (PropertyHandler h : 
ServiceLoader.load(PropertyHandler.class)) {
+propertyHandlers.add(h);
--- End diff --

I think of it the other way around: Why use two blocks when you could use 
one?


> Introduce SPI to add more column, property handlers
> ---
>
> Key: DBUTILS-124
> URL: https://issues.apache.org/jira/browse/DBUTILS-124
> Project: Commons DbUtils
>  Issue Type: New Feature
>Reporter: Carl Hall
>Assignee: Carl Hall
> Fix For: 1.7
>
>
> The column types and property types handled by {{BeanProcessor}} are hard 
> coded to the processor.  We already use a common return type, so we could add 
> a services approach using the spi built into the jdk.  This should also allow 
> other types to be handled outside of {{commons-dbutils}}.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (DBUTILS-124) Introduce SPI to add more column, property handlers

2017-10-10 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/DBUTILS-124?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16199166#comment-16199166
 ] 

ASF GitHub Bot commented on DBUTILS-124:


Github user hdevalke commented on a diff in the pull request:

https://github.com/apache/commons-dbutils/pull/3#discussion_r143818665
  
--- Diff: src/main/java/org/apache/commons/dbutils/BeanProcessor.java ---
@@ -65,19 +65,21 @@
  */
 private static final Map primitiveDefaults = new 
HashMap();
 
-/**
- * ServiceLoader to find ColumnHandler implementations on 
the classpath.  The iterator for this is
- * lazy and each time iterator() is called.
- */
-// FIXME: I think this instantiates new handlers on each iterator() 
call. This might be worth caching upfront.
-private static final ServiceLoader columnHandlers = 
ServiceLoader.load(ColumnHandler.class);
+private static final List columnHandlers = new 
ArrayList();
 
-/**
- * ServiceLoader to find PropertyHandler implementations 
on the classpath.  The iterator for this is
- * lazy and each time iterator() is called.
- */
-// FIXME: I think this instantiates new handlers on each iterator() 
call. This might be worth caching upfront.
-private static final ServiceLoader propertyHandlers = 
ServiceLoader.load(PropertyHandler.class);
+static{
+for (ColumnHandler h : ServiceLoader.load(ColumnHandler.class)) {
+columnHandlers.add(h);
+}
+}
+
+private static final List propertyHandlers = new 
ArrayList();
+
+static {
+for (PropertyHandler h : 
ServiceLoader.load(PropertyHandler.class)) {
+propertyHandlers.add(h);
--- End diff --

Can you explain why that is problematic?


> Introduce SPI to add more column, property handlers
> ---
>
> Key: DBUTILS-124
> URL: https://issues.apache.org/jira/browse/DBUTILS-124
> Project: Commons DbUtils
>  Issue Type: New Feature
>Reporter: Carl Hall
>Assignee: Carl Hall
> Fix For: 1.7
>
>
> The column types and property types handled by {{BeanProcessor}} are hard 
> coded to the processor.  We already use a common return type, so we could add 
> a services approach using the spi built into the jdk.  This should also allow 
> other types to be handled outside of {{commons-dbutils}}.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (DBUTILS-124) Introduce SPI to add more column, property handlers

2017-10-10 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/DBUTILS-124?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16199145#comment-16199145
 ] 

ASF GitHub Bot commented on DBUTILS-124:


Github user garydgregory commented on a diff in the pull request:

https://github.com/apache/commons-dbutils/pull/3#discussion_r143814271
  
--- Diff: src/main/java/org/apache/commons/dbutils/BeanProcessor.java ---
@@ -65,19 +65,21 @@
  */
 private static final Map primitiveDefaults = new 
HashMap();
 
-/**
- * ServiceLoader to find ColumnHandler implementations on 
the classpath.  The iterator for this is
- * lazy and each time iterator() is called.
- */
-// FIXME: I think this instantiates new handlers on each iterator() 
call. This might be worth caching upfront.
-private static final ServiceLoader columnHandlers = 
ServiceLoader.load(ColumnHandler.class);
+private static final List columnHandlers = new 
ArrayList();
 
-/**
- * ServiceLoader to find PropertyHandler implementations 
on the classpath.  The iterator for this is
- * lazy and each time iterator() is called.
- */
-// FIXME: I think this instantiates new handlers on each iterator() 
call. This might be worth caching upfront.
-private static final ServiceLoader propertyHandlers = 
ServiceLoader.load(PropertyHandler.class);
+static{
+for (ColumnHandler h : ServiceLoader.load(ColumnHandler.class)) {
+columnHandlers.add(h);
+}
+}
+
+private static final List propertyHandlers = new 
ArrayList();
+
+static {
+for (PropertyHandler h : 
ServiceLoader.load(PropertyHandler.class)) {
+propertyHandlers.add(h);
--- End diff --

What not use a single static block?


> Introduce SPI to add more column, property handlers
> ---
>
> Key: DBUTILS-124
> URL: https://issues.apache.org/jira/browse/DBUTILS-124
> Project: Commons DbUtils
>  Issue Type: New Feature
>Reporter: Carl Hall
>Assignee: Carl Hall
> Fix For: 1.7
>
>
> The column types and property types handled by {{BeanProcessor}} are hard 
> coded to the processor.  We already use a common return type, so we could add 
> a services approach using the spi built into the jdk.  This should also allow 
> other types to be handled outside of {{commons-dbutils}}.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (DBUTILS-124) Introduce SPI to add more column, property handlers

2017-10-10 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/DBUTILS-124?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16198985#comment-16198985
 ] 

ASF GitHub Bot commented on DBUTILS-124:


Github user thecarlhall commented on the issue:

https://github.com/apache/commons-dbutils/pull/3
  
Thanks, @hdevalke!  Nice catch.  I totally missed that `ServiceLoader` 
isn't thread safe.  I'll work to get this merged into master and cut a new 
release as soon as I can.


> Introduce SPI to add more column, property handlers
> ---
>
> Key: DBUTILS-124
> URL: https://issues.apache.org/jira/browse/DBUTILS-124
> Project: Commons DbUtils
>  Issue Type: New Feature
>Reporter: Carl Hall
>Assignee: Carl Hall
> Fix For: 1.7
>
>
> The column types and property types handled by {{BeanProcessor}} are hard 
> coded to the processor.  We already use a common return type, so we could add 
> a services approach using the spi built into the jdk.  This should also allow 
> other types to be handled outside of {{commons-dbutils}}.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (DBUTILS-124) Introduce SPI to add more column, property handlers

2017-10-10 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/DBUTILS-124?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16198473#comment-16198473
 ] 

ASF GitHub Bot commented on DBUTILS-124:


GitHub user hdevalke opened a pull request:

https://github.com/apache/commons-dbutils/pull/3

Fixes a thread safety problem introduced by DBUTILS-124.

ColumnHandlers and PropertyHandlers are preloaded in a list as the 
ServiceLoader instances are not thread safe

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/hdevalke/commons-dbutils master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/commons-dbutils/pull/3.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #3


commit 59d114082708ade64c9f05b13128d0c0eb39bc7f
Author: Hannes De Valkeneer 
Date:   2017-10-05T14:05:18Z

Fixes a thread safety problem introduced by DBUTILS-124.

ColumnHandlers and PropertyHandlers are preloaded in a list as the 
ServiceLoader instances are not thread safe




> Introduce SPI to add more column, property handlers
> ---
>
> Key: DBUTILS-124
> URL: https://issues.apache.org/jira/browse/DBUTILS-124
> Project: Commons DbUtils
>  Issue Type: New Feature
>Reporter: Carl Hall
>Assignee: Carl Hall
> Fix For: 1.7
>
>
> The column types and property types handled by {{BeanProcessor}} are hard 
> coded to the processor.  We already use a common return type, so we could add 
> a services approach using the spi built into the jdk.  This should also allow 
> other types to be handled outside of {{commons-dbutils}}.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (DBUTILS-124) Introduce SPI to add more column, property handlers

2015-04-17 Thread Carl Hall (JIRA) 

[ 
https://issues.apache.org/jira/browse/DBUTILS-124?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14500599#comment-14500599
 ] 

Carl Hall commented on DBUTILS-124:
---

I have worked this out in a local branch that introduces {{ColumnHandler}} and 
{{PropertyHandler}} interfaces and uses this with spi to dynamically load the 
same column and property handlers as was hard coded.  All tests are passing but 
I'd like to add a few more tests to show extensibility before I commit this 
upstream.

 Introduce SPI to add more column, property handlers
 ---

 Key: DBUTILS-124
 URL: https://issues.apache.org/jira/browse/DBUTILS-124
 Project: Commons DbUtils
  Issue Type: New Feature
Reporter: Carl Hall
Assignee: Carl Hall

 The column types and property types handled by {{BeanProcessor}} are hard 
 coded to the processor.  We already use a common return type, so we could add 
 a services approach using the spi built into the jdk.  This should also allow 
 other types to be handled outside of {{commons-dbutils}}.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)