You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by do...@apache.org on 2010/02/14 23:41:19 UTC

svn commit: r910110 - in /ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion: ConverterCreater.java Converters.java

Author: doogie
Date: Sun Feb 14 22:41:19 2010
New Revision: 910110

URL: http://svn.apache.org/viewvc?rev=910110&view=rev
Log:
Implement a creation framework; this allows creaters to provide
optimized versions of converters.

Added:
    ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/ConverterCreater.java
Modified:
    ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/Converters.java

Added: ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/ConverterCreater.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/ConverterCreater.java?rev=910110&view=auto
==============================================================================
--- ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/ConverterCreater.java (added)
+++ ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/ConverterCreater.java Sun Feb 14 22:41:19 2010
@@ -0,0 +1,34 @@
+/*******************************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *******************************************************************************/
+package org.ofbiz.base.conversion;
+
+/** ConverterCreater interface. Classes implement this interface to create a
+ * converter that can convert one Java object type to another.
+ */
+public interface ConverterCreater {
+    /** Creates a Converter that can convert the <code>sourceClass</code> to
+     * the <code>targetClass</code>. Returns <code>null</code> if this creater
+     * doesn't support the class pair.
+     *
+     * @param sourceClass The source <code>Class</code> to convert
+     * @param targetClass The target <code>Class</code> to convert to
+     * @return a converter that can convert <code>Object</code>s
+     */
+    public <S, T> Converter<S, T> createConverter(Class<S> sourceClass, Class<T> targetClass);
+}

Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/Converters.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/Converters.java?rev=910110&r1=910109&r2=910110&view=diff
==============================================================================
--- ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/Converters.java (original)
+++ ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/Converters.java Sun Feb 14 22:41:19 2010
@@ -38,6 +38,7 @@
     protected static final String module = Converters.class.getName();
     protected static final String DELIMITER = "->";
     protected static final FastMap<String, Converter<?, ?>> converterMap = FastMap.newInstance();
+    protected static final FastSet<ConverterCreater> creaters = FastSet.newInstance();
     protected static final FastSet<String> noConversions = FastSet.newInstance();
     /** Null converter used when the source and target java object
      * types are the same. The <code>convert</code> method returns the
@@ -48,6 +49,7 @@
 
     static {
         converterMap.setShared(true);
+        registerCreater(new PassThruConverterCreater());
         ClassLoader loader = Thread.currentThread().getContextClassLoader();
         Iterator<ConverterLoader> converterLoaders = ServiceRegistry.lookupProviders(ConverterLoader.class, loader);
         while (converterLoaders.hasNext()) {
@@ -97,11 +99,12 @@
                     continue OUTER;
                 }
             }
-            // Null converter must be checked last
-            if (nullConverter.canConvert(sourceClass, targetClass)) {
-                Converter passThruConverter = new PassThruConverter<S>(sourceClass);
-                converterMap.putIfAbsent(key, passThruConverter);
-                continue;
+            for (ConverterCreater value : creaters) {
+                result = createConverter(value, sourceClass, targetClass);
+                if (result != null) {
+                    converterMap.putIfAbsent(key, result);
+                    continue OUTER;
+                }
             }
             if (noConversions.add(key)) {
                 Debug.logWarning("*** No converter found, converting from " +
@@ -113,6 +116,10 @@
         } while (true);
     }
 
+    private static <S, SS extends S, T, TT extends T> Converter<SS, TT> createConverter(ConverterCreater creater, Class<SS> sourceClass, Class<TT> targetClass) {
+        return creater.createConverter(sourceClass, targetClass);
+    }
+
     /** Load all classes that implement <code>Converter</code> and are
      * contained in <code>containerClass</code>.
      *
@@ -143,6 +150,18 @@
         }
     }
 
+    /** Registers a <code>ConverterCreater</code> instance to be used by the
+     * {@link org.ofbiz.base.conversion.Converters#getConverter(Class, Class)}
+     * method, when a converter can't be found.
+     *
+     * @param <S> The source object type
+     * @param <T> The target object type
+     * @param creater The <code>ConverterCreater</code> instance to register
+     */
+    public static <S, T> void registerCreater(ConverterCreater creater) {
+        creaters.add(creater);
+    }
+
     /** Registers a <code>Converter</code> instance to be used by the
      * {@link org.ofbiz.base.conversion.Converters#getConverter(Class, Class)}
      * method.
@@ -209,6 +228,19 @@
         }
     }
 
+    protected static class PassThruConverterCreater implements ConverterCreater{
+        protected PassThruConverterCreater() {
+        }
+
+        public <S, T> Converter<S, T> createConverter(Class<S> sourceClass, Class<T> targetClass) {
+            if (sourceClass == targetClass) {
+                return UtilGenerics.cast(new PassThruConverter<T>(targetClass));
+            } else {
+                return null;
+            }
+        }
+    }
+
     /** Pass thru converter used when the source and target java object
      * types are the same. The <code>convert</code> method returns the
      * source object.



Re: svn commit: r910110 - in /ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion: ConverterCreater.java Converters.java

Posted by Adrian Crum <ad...@hlmksw.com>.
Adam Heath wrote:
> doogie@apache.org wrote:
>> Author: doogie
>> Date: Sun Feb 14 22:41:19 2010
>> New Revision: 910110
>>
>> URL: http://svn.apache.org/viewvc?rev=910110&view=rev
>> Log:
>> Implement a creation framework; this allows creaters to provide
>> optimized versions of converters.
>>
>> Added:
>>     ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/ConverterCreater.java
>> Modified:
>>     ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/Converters.java
> 
> This is the commit that broke the build.
> 
> What happened here, is I had tested an earlier version of this change,
> and everything worked(localized tests in base, and full system tests).
>  I then changed the commit, and got excited about committing
> everything, so didn't go thru a full test cycle.
> 
> Ideally, going forward, to help solve this, we need to have full
> coverage of base; using cobertura, you can see what lines are being
> run.  I am currently getting the conversion system to something close
> to 100%.  If it had more coverage, this problem would have been
> discovered with the simple tests.


I thought it was the misspelling of creators that broke the build. Thank 
you for the clarification.

Re: svn commit: r910110 - in /ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion: ConverterCreater.java Converters.java

Posted by Adam Heath <do...@brainfood.com>.
>> What happened here, is I had tested an earlier version of this change,
>> and everything worked(localized tests in base, and full system tests).
>> I then changed the commit, and got excited about committing
>> everything, so didn't go thru a full test cycle.
>>
>> Ideally, going forward, to help solve this, we need to have full
>> coverage of base; using cobertura, you can see what lines are being
>> run.  I am currently getting the conversion system to something close
>> to 100%.  If it had more coverage, this problem would have been
>> discovered with the simple tests.
> 
> I think I'm missing something here... how does this relate to a failure to run tests?

I guess you didn't read the first paragraph I wrote.

It doesn't.  But in this particular case, if the original code *was*
tested, both the line, *and* all branch points, then the test cases
would have failed when I changed the code.  This is only applicable
for this particular bug that I introduced.

==
if (foo || bar || baz > 1) {
==

cobertura is smart enough to see that the line is covered, yes, but
that the bar and baz parts are not.

> Another small and insignificant detail to consider is that: 100% coverage does not mean 100% bug-free, unless perhaps you are a marketing professional.

Never said that 100% coverage means you've handled all cases.  But if
you do *not* have 100% coverage, you *know*, beyond a shadow of a
doubt, that there are things not being tested.


Re: svn commit: r910110 - in /ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion: ConverterCreater.java Converters.java

Posted by David E Jones <de...@me.com>.
On Feb 15, 2010, at 10:22 AM, Adam Heath wrote:

> doogie@apache.org wrote:
>> Author: doogie
>> Date: Sun Feb 14 22:41:19 2010
>> New Revision: 910110
>> 
>> URL: http://svn.apache.org/viewvc?rev=910110&view=rev
>> Log:
>> Implement a creation framework; this allows creaters to provide
>> optimized versions of converters.
>> 
>> Added:
>>    ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/ConverterCreater.java
>> Modified:
>>    ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/Converters.java
> 
> This is the commit that broke the build.
> 
> What happened here, is I had tested an earlier version of this change,
> and everything worked(localized tests in base, and full system tests).
> I then changed the commit, and got excited about committing
> everything, so didn't go thru a full test cycle.
> 
> Ideally, going forward, to help solve this, we need to have full
> coverage of base; using cobertura, you can see what lines are being
> run.  I am currently getting the conversion system to something close
> to 100%.  If it had more coverage, this problem would have been
> discovered with the simple tests.

I think I'm missing something here... how does this relate to a failure to run tests?

Another small and insignificant detail to consider is that: 100% coverage does not mean 100% bug-free, unless perhaps you are a marketing professional.

-David


Re: svn commit: r910110 - in /ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion: ConverterCreater.java Converters.java

Posted by Adam Heath <do...@brainfood.com>.
doogie@apache.org wrote:
> Author: doogie
> Date: Sun Feb 14 22:41:19 2010
> New Revision: 910110
> 
> URL: http://svn.apache.org/viewvc?rev=910110&view=rev
> Log:
> Implement a creation framework; this allows creaters to provide
> optimized versions of converters.
> 
> Added:
>     ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/ConverterCreater.java
> Modified:
>     ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/Converters.java

This is the commit that broke the build.

What happened here, is I had tested an earlier version of this change,
and everything worked(localized tests in base, and full system tests).
 I then changed the commit, and got excited about committing
everything, so didn't go thru a full test cycle.

Ideally, going forward, to help solve this, we need to have full
coverage of base; using cobertura, you can see what lines are being
run.  I am currently getting the conversion system to something close
to 100%.  If it had more coverage, this problem would have been
discovered with the simple tests.