Discussion:
svn commit: r1766027 - in /turbine/fulcrum/trunk/intake: ./ src/java/org/apache/fulcrum/intake/model/ src/test/org/apache/fulcrum/intake/
Thomas Vandahl
2016-10-21 15:55:08 UTC
Permalink
On 21.10.16 16:09, ***@apache.org wrote:
> Author: gk
> Date: Fri Oct 21 14:09:09 2016
> New Revision: 1766027
>
> URL: http://svn.apache.org/viewvc?rev=1766027&view=rev
> Log:
> TRB-99 fixed -tested ok: jdk 1.7 and 1.8

Whoo, that's quite a lot of changes. Could you please explain a bit what
you did and why? Some things look strange to me.

Bye, Thomas.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-***@turbine.apache.org
For additional commands, e-mail: dev-***@turbine.apache.org
Thomas Vandahl
2016-10-23 17:23:44 UTC
Permalink
Hi Georg,

On 21.10.16 16:09, ***@apache.org wrote:
> Author: gk
> Date: Fri Oct 21 14:09:09 2016
> New Revision: 1766027
>
> URL: http://svn.apache.org/viewvc?rev=1766027&view=rev
> Log:
> TRB-99 fixed -tested ok: jdk 1.7 and 1.8
[...]
> Modified: turbine/fulcrum/trunk/intake/src/java/org/apache/fulcrum/intake/model/FieldAdapter.java
> URL: http://svn.apache.org/viewvc/turbine/fulcrum/trunk/intake/src/java/org/apache/fulcrum/intake/model/FieldAdapter.java?rev=1766027&r1=1766026&r2=1766027&view=diff
> ==============================================================================
> --- turbine/fulcrum/trunk/intake/src/java/org/apache/fulcrum/intake/model/FieldAdapter.java (original)
> +++ turbine/fulcrum/trunk/intake/src/java/org/apache/fulcrum/intake/model/FieldAdapter.java Fri Oct 21 14:09:09 2016
> @@ -20,7 +20,10 @@ package org.apache.fulcrum.intake.model;
> */
>
> import java.lang.reflect.Constructor;
> +import java.util.ArrayList;
> +import java.util.Arrays;
> import java.util.HashMap;
> +import java.util.List;
> import java.util.Map;
>
> import javax.xml.bind.annotation.adapters.XmlAdapter;
> @@ -210,6 +213,18 @@ public class FieldAdapter extends XmlAda
> FieldCtor fieldCtor = null;
> Field<?> field = null;
> String type = xmlField.getType();
> +
> + List<Rule> rules = xmlField.getRules();
> + if (rules != null )
> + {
> +
> + xmlField.getRuleMap().clear();
> + Map<String,Rule> ruleMap = xmlField.getRuleMap();
> + for (Rule rule : rules)
> + {
> + ruleMap.put(rule.getName(), rule);
> + }
> + }

I suggest to keep the adapter free of these operations. I'd rather stick
to afterUnmarshal() to complete the data objects. The following is not
necessary then and the scope is kept locally.

>
> fieldCtor = fieldCtors.get(type);
> if (fieldCtor == null)
> @@ -231,9 +246,37 @@ public class FieldAdapter extends XmlAda
> @Override
> public Field<?> unmarshal(XmlField xmlField) throws Exception
> {
> - return getInstance(xmlField, xmlField.getGroup());
> + Field<?> field = getInstance(xmlField, xmlField.getGroup());
> +
> + processDataInGroupContext(xmlField, field);
> +
> + return field;
[...]

+1 for switching to JUnit 4.

Would you like to see my approach?

Bye, Thomas.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-***@turbine.apache.org
For additional commands, e-mail: dev-***@turbine.apache.org
Georg Kallidis
2016-10-24 05:55:26 UTC
Permalink
Hi Thomas,

@my approach
I started with updating the deps, and upgrade to JUnit4.

JAXB before calling afterUnMarshal did not visit the code in Group.setFields. I researched a little bit (Stackoverflow..) and thought the best hook would be the Adapter, which already exists.
As it hooks the instantiation of a single field, I had to change a little bit the update of the arrays/map. The same was true for the rules, but they are available in the adapter ready as a list, so I could just copy and past the code.

@your approach
Please feel free to discuss or change it in the code, if it could be done in afterUnMarshal may be the better!

-Georg


-----Thomas Vandahl <***@apache.org> schrieb: -----
An: ***@turbine.apache.org
Von: Thomas Vandahl <***@apache.org>
Datum: 23.10.2016 19:23
Betreff: Re: svn commit: r1766027 - in /turbine/fulcrum/trunk/intake: ./ src/java/org/apache/fulcrum/intake/model/ src/test/org/apache/fulcrum/intake/

Hi Georg,

On 21.10.16 16:09, ***@apache.org wrote:
> Author: gk
> Date: Fri Oct 21 14:09:09 2016
> New Revision: 1766027
>
> URL: http://svn.apache.org/viewvc?rev=1766027&view=rev
> Log:
> TRB-99 fixed -tested ok: jdk 1.7 and 1.8
[...]
> Modified: turbine/fulcrum/trunk/intake/src/java/org/apache/fulcrum/intake/model/FieldAdapter.java
> URL: http://svn.apache.org/viewvc/turbine/fulcrum/trunk/intake/src/java/org/apache/fulcrum/intake/model/FieldAdapter.java?rev=1766027&r1=1766026&r2=1766027&view=diff
> ==============================================================================
> --- turbine/fulcrum/trunk/intake/src/java/org/apache/fulcrum/intake/model/FieldAdapter.java (original)
> +++ turbine/fulcrum/trunk/intake/src/java/org/apache/fulcrum/intake/model/FieldAdapter.java Fri Oct 21 14:09:09 2016
> @@ -20,7 +20,10 @@ package org.apache.fulcrum.intake.model;
>   */
>  
>  import java.lang.reflect.Constructor;
> +import java.util.ArrayList;
> +import java.util.Arrays;
>  import java.util.HashMap;
> +import java.util.List;
>  import java.util.Map;
>  
>  import javax.xml.bind.annotation.adapters.XmlAdapter;
> @@ -210,6 +213,18 @@ public class FieldAdapter extends XmlAda
>          FieldCtor fieldCtor = null;
>          Field<?> field = null;
>          String type = xmlField.getType();
> +        
> +        List<Rule> rules = xmlField.getRules();
> +        if (rules != null )
> +        {
> +        
> +         xmlField.getRuleMap().clear();
> +         Map<String,Rule> ruleMap = xmlField.getRuleMap();
> +         for (Rule rule : rules)
> +        {
> +            ruleMap.put(rule.getName(), rule);
> +        }
> +        }

I suggest to keep the adapter free of these operations. I'd rather stick
to afterUnmarshal() to complete the data objects. The following is not
necessary then and the scope is kept locally.

>  
>          fieldCtor = fieldCtors.get(type);
>          if (fieldCtor == null)
> @@ -231,9 +246,37 @@ public class FieldAdapter extends XmlAda
>      @Override
>      public Field<?> unmarshal(XmlField xmlField) throws Exception
>      {
> -        return getInstance(xmlField, xmlField.getGroup());
> +     Field<?> field = getInstance(xmlField, xmlField.getGroup());
> +    
> +     processDataInGroupContext(xmlField, field);        
> +    
> +     return field;
[...]

+1 for switching to JUnit 4.

Would you like to see my approach?

Bye, Thomas.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-***@turbine.apache.org
For additional commands, e-mail: dev-***@turbine.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-***@turbine.apache.org
For additional commands, e-ma
Loading...