Modello

Wrong hanling of Map field in XPP3Reader/Writer with multiply association

Details

  • Type: Bug Bug
  • Status: Open Open
  • Priority: Critical Critical
  • Resolution: Unresolved
  • Affects Version/s: 1.0.2, 1.1
  • Fix Version/s: None
  • Component/s: modello-plugin-xpp3
  • Labels:
    None
  • Environment:
    maven 2.2.0 java 1.6.0_13
  • Complexity:
    Intermediate
  • Number of attachments :
    2

Description

It convince in the model to define a field as a Map with some stringable id as a key and an association of the target class

Modello xml snippet

    <class xml.tagName="agilepom" rootElement="true">
      <name>AgilePOM</name>
      <description>
        <![CDATA[
The <code>&lt;agileProject&gt;</code> element is the root of the descriptor.
The following table lists all of the possible child elements.
        ]]>
      </description>
      <version>1.0.0+</version>
      <fields>
...
        <field xdoc.separator="blank">
          <name>developers</name>
          <version>1.0.0+</version>
          <description>Describes the committers of a project.</description>
          <type>Map</type>
          <association>
            <type>Developer</type>
            <multiplicity>*</multiplicity>
          </association>
        </field>
 ...
    </class>

    
C> mvn clean package

[INFO] Scanning for projects...
[INFO] ------------------------------------------------------------------------
[INFO] Building dbsvn-model
[INFO]    task-segment: [clean, package]
[INFO] ------------------------------------------------------------------------
[INFO] [clean:clean {execution: default-clean}]
[INFO] Deleting directory C:\dev\org\tigris\dbsvn\trunk\home\dbsvn-model\target
[INFO] [modello:java {execution: java-sources}]
[INFO] outputDirectory: C:\dev\org\tigris\dbsvn\trunk\home\dbsvn-model\target\generated-sources\modello
[INFO] Working on model: src/main/mdo/AgilePOM.mdo
[INFO] Generating current version: 1.0.0
[INFO] Working on model: src/main/mdo/Dbsvn.mdo
[INFO] Generating current version: 1.0.0
[INFO] [modello:xpp3-writer {execution: java-sources}]
[INFO] outputDirectory: C:\dev\org\tigris\dbsvn\trunk\home\dbsvn-model\target\generated-sources\modello
[INFO] Working on model: src/main/mdo/AgilePOM.mdo
[INFO] Generating current version: 1.0.0
[INFO] Working on model: src/main/mdo/Dbsvn.mdo
[INFO] Generating current version: 1.0.0
[INFO] [modello:xpp3-reader {execution: java-sources}]
[INFO] outputDirectory: C:\dev\org\tigris\dbsvn\trunk\home\dbsvn-model\target\generated-sources\modello
[INFO] Working on model: src/main/mdo/AgilePOM.mdo
[INFO] Generating current version: 1.0.0
[INFO] Working on model: src/main/mdo/Dbsvn.mdo
[INFO] Generating current version: 1.0.0
[INFO] [resources:resources {execution: default-resources}]
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] Copying 0 resource
[INFO] [compiler:compile {execution: default-compile}]
[INFO] Compiling 18 source files to C:\dev\org\tigris\dbsvn\trunk\home\dbsvn-model\target\classes
[INFO] ------------------------------------------------------------------------
[ERROR] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Compilation failure
C:\dev\org\tigris\dbsvn\trunk\home\dbsvn-model\target\generated-sources\modello\org\tigris\dbsvn\model\agilepom\io\xpp3\AgilePOMXpp3Reader.java:[483,32] addDeveloper(java.lang.Object,org.tigris.dbsvn.model.agilepom.Developer) in org.tigris.dbsvn.model.agilepom.AgilePOM cannot be applied to (java.lang.String,java.lang.String)


[INFO] ------------------------------------------------------------------------
[INFO] For more information, run Maven with the -e switch
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 4 seconds
[INFO] Finished at: Thu Jul 09 09:49:22 GMT+01:00 2009
[INFO] Final Memory: 14M/27M
[INFO] ------------------------------------------------------------------------

Looking in the generated sources
......................./dbsvn-model/target/generated-sources/modello/org/tigris/dbsvn/model/agilepom/AgilePOM.java

public class AgilePOM
    implements java.io.Serializable
{
...
    /**
     * Field developers.
     */
    private java.util.Map developers;
...
    /**
     * Method getDevelopers.
     * 
     * @return Map
     */
    public java.util.Map getDevelopers()
    {
        if ( this.developers == null )
        {
            this.developers = new java.util.HashMap();
        }

        return this.developers;
    } //-- java.util.Map getDevelopers()
...

    /**
     * Set describes the commiters of a project.
     * 
     * @param developers
     */
    public void setDevelopers( java.util.Map developers )
    {
        this.developers = developers;
    } //-- void setDevelopers( java.util.Map )

...
    /**
     * Method addDeveloper.
     * 
     * @param key
     * @param value
     */
    public void addDeveloper( Object key, Developer value )
    {
        getDevelopers().put( key, value );
    } //-- void addDeveloper( Object, Developer )
...

............./dbsvn-model/target/generated-sources/modello/org/tigris/dbsvn/model/agilepom/io/xpp3/AgilePOMXpp3Reader.java

                else if ( checkFieldWithDuplicate( parser, "developers", null, parsed ) )
                {
                    while ( parser.nextTag() == XmlPullParser.START_TAG )
                    {
                        String key = parser.getName();
                        String value = parser.nextText().trim();
                        agilePOM.addDeveloper( key, value );
                    }
                }

............org.tigris.dbsvn.model.agilepom.io.xpp3.AgilePOMXpp3Writer

**
 * Class AgilePOMXpp3Writer.
 * 
 * @version $Revision$ $Date$
 */
public class AgilePOMXpp3Writer
{
...
    /**
     * Method writeAgilePOM.
     * 
     * @param agilePOM
     * @param serializer
     * @param tagName
     * @throws java.io.IOException
     */
    private void writeAgilePOM( AgilePOM agilePOM, String tagName, XmlSerializer serializer )
        throws java.io.IOException
    {
        if ( agilePOM != null )
        {
...
            if ( ( agilePOM.getDevelopers() != null ) && ( agilePOM.getDevelopers().size() > 0 ) )
            {
                serializer.startTag( NAMESPACE, "developers" );
                for ( Iterator iter = agilePOM.getDevelopers().keySet().iterator(); iter.hasNext(); )
                {
                    String key = (String) iter.next();
                    String value = (String) agilePOM.getDevelopers().get( key );
                    serializer.startTag( NAMESPACE, "" + key + "" ).text( value ).endTag( NAMESPACE, "" + key + "" );
                }
                serializer.endTag( NAMESPACE, "developers" );
            }
...
}

Conclusion
~~~~~~~~~
XPP3 plugin correctly generate AgilePOM#addDeveloper(Object, Developer)
(I think here AgilePOM#addDeveloper(String, Developer) suit better though, but OK)

However both Reader and Writer generate wrong snippets

                    String key = (String) iter.next();
                    String value = (String) agilePOM.getDevelopers().get( key );

I think it could be (for Writer)

                    Object key = iter.next();
                    Developer developer = (Developer) agilePOM.getDevelopers().get( key );
                    write(o,"key", key)
                    writeDeveloper( o, "developer", developer, serializer );

and correspond snippet for Reader

                     String key = parseKey("key", parser)
                     developers.addDeveloper( key, parseDeveloper( "developer", parser, strict ) );

xml should be look as 

<developers>
  <key>taranenko</key>
  <developer>
    <primaryRole>engineer</primaryRole>
    ...
  </developer>
</developers>
  1. File
    features.mdo
    09/Jul/09 5:51 AM
    49 kB
    Oleg Taranenko
  2. Text File
    map-assosiation.patch
    17/Jul/09 4:08 PM
    9 kB
    Oleg Taranenko

Issue Links

is related to

Bug - A problem which impairs or prevents the functions of the product. MODELLO-30 MAP generation requires stash.keyType attribute

  • Major - Major loss of function.
  • Open - The issue is open and ready for the assignee to start work on it.

Bug - A problem which impairs or prevents the functions of the product. MODELLO-62 Map With association and defaultValue Does Not Instantiate

  • Major - Major loss of function.
  • Open - The issue is open and ready for the assignee to start work on it.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Oleg Taranenko added a comment - - edited

As an option suggest format of the xml

<developers>
  <developer key="taranenko">
     <primaryRole>engineer</primaryRole>
...
   </developer>
</developers>
Show
Oleg Taranenko added a comment - - edited As an option suggest format of the xml <developers> <developer key= "taranenko" > <primaryRole> engineer </primaryRole> ... </developer> </developers>
Hide
Permalink
Oleg Taranenko added a comment -

It will break features.mdo JUnit test

Show
Oleg Taranenko added a comment - It will break features.mdo JUnit test
Hide
Permalink
Oleg Taranenko added a comment - - edited

I've made the draft changes, which implemented required functionality. Of course, it is incomplete and supports only xpp3 reader and writer plugins, other not. If require, I could accomplish for other plugins, but now I'm using only xpp,

plugin assume (hardcoded), that asssociated class must have a field with name 'key' and make use it as a maps key. Map's value is the instance of the target class.

It produce code for xpp reader

                else if ( checkFieldWithDuplicate( parser, "roles", null, parsed ) )
                {
                    java.util.Map roles = new java.util.HashMap();
                    agilePOM.setRoles( roles );
                    while ( parser.nextTag() == XmlPullParser.START_TAG )
                    {
                        if ( parser.getName().equals( "role" ) )
                        {
                            Role role = parseRole( "role", parser, strict );
                            String key = role.getKey();
                            Role exists = (Role) roles.put( key, role );
                            if ( strict && exists != null ) {
                                throw new XmlPullParserException( "Map item with key '" + key + "' already added" );
                            }
                        }
                        else if ( strict )
                        {
                            throw new XmlPullParserException( "Unrecognised association: '" + parser.getName() + "'", parser, null );
                        }
                        else
                        {
                            // swallow up to end tag since this is not valid
                            while ( parser.next() != XmlPullParser.END_TAG ) {}
                        }
                    }
                }

and for xpp writer

            if ( ( agilePOM.getRoles() != null ) && ( agilePOM.getRoles().size() > 0 ) )
            {
                serializer.startTag( NAMESPACE, "roles" );
                for ( Iterator iter = agilePOM.getRoles().entrySet().iterator(); iter.hasNext(); )
                {
                    java.util.Map.Entry e = (java.util.Map.Entry) iter.next();
                    Role o = (Role) e.getValue();
                    if (o.getKey() == null)
                    {
                        o.setKey((String) e.getKey());
                    }
                    writeRole( o, "role", serializer );
                }
                serializer.endTag( NAMESPACE, "roles" );
            }

No objection however to add to modello Model an attribute for association tag to define which field should be used as a key.

        <field>
          <name>roles</name>
          <description>Describes all roles participation to a project.</description>
          <type>Map</type>
          <association java.mapKeyAttribute="role">
            <type>Role</type>
            <multiplicity>*</multiplicity>
          </association>
        </field>

and produce getRole() and setRole() methods instead of getKey()/setKey() in the Associated Class

Show
Oleg Taranenko added a comment - - edited I've made the draft changes, which implemented required functionality. Of course, it is incomplete and supports only xpp3 reader and writer plugins, other not. If require, I could accomplish for other plugins, but now I'm using only xpp, plugin assume (hardcoded), that asssociated class must have a field with name 'key' and make use it as a maps key. Map's value is the instance of the target class. It produce code for xpp reader else if ( checkFieldWithDuplicate( parser, "roles" , null , parsed ) ) { java.util.Map roles = new java.util.HashMap(); agilePOM.setRoles( roles ); while ( parser.nextTag() == XmlPullParser.START_TAG ) { if ( parser.getName().equals( "role" ) ) { Role role = parseRole( "role" , parser, strict ); String key = role.getKey(); Role exists = (Role) roles.put( key, role ); if ( strict && exists != null ) { throw new XmlPullParserException( "Map item with key '" + key + "' already added" ); } } else if ( strict ) { throw new XmlPullParserException( "Unrecognised association: '" + parser.getName() + "'" , parser, null ); } else { // swallow up to end tag since this is not valid while ( parser.next() != XmlPullParser.END_TAG ) {} } } } and for xpp writer if ( ( agilePOM.getRoles() != null ) && ( agilePOM.getRoles().size() > 0 ) ) { serializer.startTag( NAMESPACE, "roles" ); for ( Iterator iter = agilePOM.getRoles().entrySet().iterator(); iter.hasNext(); ) { java.util.Map.Entry e = (java.util.Map.Entry) iter.next(); Role o = (Role) e.getValue(); if (o.getKey() == null ) { o.setKey(( String ) e.getKey()); } writeRole( o, "role" , serializer ); } serializer.endTag( NAMESPACE, "roles" ); } No objection however to add to modello Model an attribute for association tag to define which field should be used as a key. <field> <name>roles</name> <description>Describes all roles participation to a project.</description> <type>Map</type> <association java.mapKeyAttribute= "role" > <type>Role</type> <multiplicity>*</multiplicity> </association> </field> and produce getRole() and setRole() methods instead of getKey()/setKey() in the Associated Class
Hide
Permalink
Herve Boutemy added a comment -

I'm convinced like you that Map support is actually broken: I doubt anybody has really used it.

Your proposal with java.mapKeyAttribute sounds good. It could support more than simple String as Map's key: the effective type of the field used defines key's type, which will be written as generics in the Map's declaration.

I have one concern: what to do in the Role.setKey() method to refresh the Map? Should really the Map's key values be in a field of the associated class?

If I try another point of view: from xml.mapStyle feature, I suppose work on Properties support was done, and rapidly promoted as Map since Properties are like Map<String, String>. To fix Map support consistently with Properties, key should be a String, either inline or exploded, not available in association class.

<inlineMaps>
  <the key><role attr="">...</role></the key>
  ...
</inlineMaps>
<explodeMaps>
  <explodeMap>
    <key>the key</key>
    <role attr="">...</role>
  </explodeMap>
  ...
</explodeMaps>

WDYT?

Show
Herve Boutemy added a comment - I'm convinced like you that Map support is actually broken: I doubt anybody has really used it. Your proposal with java.mapKeyAttribute sounds good. It could support more than simple String as Map's key: the effective type of the field used defines key's type, which will be written as generics in the Map's declaration. I have one concern: what to do in the Role.setKey() method to refresh the Map? Should really the Map's key values be in a field of the associated class? If I try another point of view: from xml.mapStyle feature, I suppose work on Properties support was done, and rapidly promoted as Map since Properties are like Map<String, String> . To fix Map support consistently with Properties, key should be a String, either inline or exploded, not available in association class. <inlineMaps> <the key> <role attr=""> ... </role> </the key> ... </inlineMaps> <explodeMaps> <explodeMap> <key> the key </key> <role attr=""> ... </role> </explodeMap> ... </explodeMaps> WDYT?

People

  • Assignee:
    Unassigned
    Reporter:
    Oleg Taranenko
Vote (0)
Watch (1)

Dates

  • Created:
    Updated: