Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

61 input location #62

Merged
merged 2 commits into from Mar 9, 2019
Merged

61 input location #62

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2d07bf7
#61 add input location support when building XPP3 DOM
hboutemy Feb 21, 2019
0eaf19a
#61 keep input location while merging
hboutemy Feb 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Expand Up @@ -26,7 +26,7 @@ limitations under the License.
</parent>

<artifactId>plexus-utils</artifactId>
<version>3.1.2-SNAPSHOT</version>
<version>3.2.0-SNAPSHOT</version>

<name>Plexus Common Utilities</name>
<description>A collection of various utility classes to ease working with strings, files, command lines, XML and
Expand Down
90 changes: 72 additions & 18 deletions src/main/java/org/codehaus/plexus/util/xml/Xpp3Dom.java
View file
Open in desktop
Expand Up @@ -48,6 +48,11 @@ public class Xpp3Dom

protected Xpp3Dom parent;

/**
* @since 3.2.0
*/
protected Object inputLocation;

private static final String[] EMPTY_STRING_ARRAY = new String[0];

private static final Xpp3Dom[] EMPTY_DOM_ARRAY = new Xpp3Dom[0];
Expand Down Expand Up @@ -86,6 +91,15 @@ public Xpp3Dom( String name )
childMap = new HashMap<String, Xpp3Dom>();
}

/**
* @since 3.2.0
*/
public Xpp3Dom( String name, Object inputLocation )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking out loud: Can we add a generic to Xpp3Dom, like Xpp3Dom<T>, where T reflects the inputLocation?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose it is feasible: IMHO, gives too much visibility to input location tracking, which is really something very special, but feasible

{
this( name );
this.inputLocation = inputLocation;
}

/**
* Copy constructor.
*/
Expand All @@ -100,6 +114,7 @@ public Xpp3Dom( Xpp3Dom src )
public Xpp3Dom( Xpp3Dom src, String name )
{
this.name = name;
this.inputLocation = src.inputLocation;

int childCount = src.getChildCount();

Expand Down Expand Up @@ -278,6 +293,26 @@ public void setParent( Xpp3Dom parent )
this.parent = parent;
}

// ----------------------------------------------------------------------
// Input location handling
// ----------------------------------------------------------------------

/**
* @since 3.2.0
*/
public Object getInputLocation()
{
return inputLocation;
}

/**
* @since 3.2.0
*/
public void setInputLocation( Object inputLocation )
{
this.inputLocation = inputLocation;
}

// ----------------------------------------------------------------------
// Helpers
// ----------------------------------------------------------------------
Expand All @@ -296,23 +331,41 @@ public void writeToSerializer( String namespace, XmlSerializer serializer )
}

/**
* Merges one DOM into another, given a specific algorithm and possible override points for that algorithm. The
* algorithm is as follows: 1. if the recessive DOM is null, there is nothing to do...return. 2. Determine whether
* the dominant node will suppress the recessive one (flag=mergeSelf). A. retrieve the 'combine.self' attribute on
* the dominant node, and try to match against 'override'... if it matches 'override', then set mergeSelf ==
* false...the dominant node suppresses the recessive one completely. B. otherwise, use the default value for
* mergeSelf, which is true...this is the same as specifying 'combine.self' == 'merge' as an attribute of the
* dominant root node. 3. If mergeSelf == true A. if the dominant root node's value is empty, set it to the
* recessive root node's value B. For each attribute in the recessive root node which is not set in the dominant
* root node, set it. C. Determine whether children from the recessive DOM will be merged or appended to the
* dominant DOM as siblings (flag=mergeChildren). i. if childMergeOverride is set (non-null), use that value
* (true/false) ii. retrieve the 'combine.children' attribute on the dominant node, and try to match against
* 'append'...if it matches 'append', then set mergeChildren == false...the recessive children will be appended as
* siblings of the dominant children. iii. otherwise, use the default value for mergeChildren, which is true...this
* is the same as specifying 'combine.children' == 'merge' as an attribute on the dominant root node. D. Iterate
* through the recessive children, and: i. if mergeChildren == true and there is a corresponding dominant child
* (matched by element name), merge the two. ii. otherwise, add the recessive child as a new child on the dominant
* root node.
* Merges one DOM into another, given a specific algorithm and possible override points for that algorithm.<p>
* The algorithm is as follows:
* <ol>
* <li> if the recessive DOM is null, there is nothing to do... return.</li>
* <li> Determine whether the dominant node will suppress the recessive one (flag=mergeSelf).
* <ol type="A">
* <li> retrieve the 'combine.self' attribute on the dominant node, and try to match against 'override'...
* if it matches 'override', then set mergeSelf == false...the dominant node suppresses the recessive one
* completely.</li>
* <li> otherwise, use the default value for mergeSelf, which is true...this is the same as specifying
* 'combine.self' == 'merge' as an attribute of the dominant root node.</li>
* </ol></li>
* <li> If mergeSelf == true
* <ol type="A">
* <li> if the dominant root node's value is empty, set it to the recessive root node's value</li>
* <li> For each attribute in the recessive root node which is not set in the dominant root node, set it.</li>
* <li> Determine whether children from the recessive DOM will be merged or appended to the dominant DOM as
* siblings (flag=mergeChildren).
* <ol type="i">
* <li> if childMergeOverride is set (non-null), use that value (true/false)</li>
* <li> retrieve the 'combine.children' attribute on the dominant node, and try to match against
* 'append'...</li>
* <li> if it matches 'append', then set mergeChildren == false...the recessive children will be appended as
* siblings of the dominant children.</li>
* <li> otherwise, use the default value for mergeChildren, which is true...this is the same as specifying
* 'combine.children' == 'merge' as an attribute on the dominant root node.</li>
* </ol></li>
* <li> Iterate through the recessive children, and:
* <ol type="i">
* <li> if mergeChildren == true and there is a corresponding dominant child (matched by element name),
* merge the two.</li>
* <li> otherwise, add the recessive child as a new child on the dominant root node.</li>
* </ol></li>
* </ol></li>
* </ol>
*/
private static void mergeIntoXpp3Dom( Xpp3Dom dominant, Xpp3Dom recessive, Boolean childMergeOverride )
{
Expand All @@ -333,9 +386,10 @@ private static void mergeIntoXpp3Dom( Xpp3Dom dominant, Xpp3Dom recessive, Boole

if ( mergeSelf )
{
if ( isEmpty( dominant.getValue() ) )
if ( isEmpty( dominant.getValue() ) && !isEmpty( recessive.getValue() ) )
{
dominant.setValue( recessive.getValue() );
dominant.setInputLocation( recessive.getInputLocation() );
}

String[] recessiveAttrs = recessive.getAttributeNames();
Expand Down
46 changes: 44 additions & 2 deletions src/main/java/org/codehaus/plexus/util/xml/Xpp3DomBuilder.java
View file
Open in desktop
Expand Up @@ -37,7 +37,16 @@ public class Xpp3DomBuilder
public static Xpp3Dom build( Reader reader )
throws XmlPullParserException, IOException
{
return build( reader, DEFAULT_TRIM );
return build( reader, null );
}

/**
* @since 3.2.0
*/
public static Xpp3Dom build( Reader reader, InputLocationBuilder locationBuilder )
throws XmlPullParserException, IOException
{
return build( reader, DEFAULT_TRIM, locationBuilder );
}

public static Xpp3Dom build( InputStream is, String encoding )
Expand Down Expand Up @@ -68,13 +77,22 @@ public static Xpp3Dom build( InputStream is, String encoding, boolean trim )

public static Xpp3Dom build( Reader reader, boolean trim )
throws XmlPullParserException, IOException
{
return build( reader, trim, null );
}

/**
* @since 3.2.0
*/
public static Xpp3Dom build( Reader reader, boolean trim, InputLocationBuilder locationBuilder )
throws XmlPullParserException, IOException
{
try
{
final XmlPullParser parser = new MXParser();
parser.setInput( reader );

final Xpp3Dom xpp3Dom = build( parser, trim );
final Xpp3Dom xpp3Dom = build( parser, trim, locationBuilder );
reader.close();
reader = null;

Expand All @@ -94,6 +112,15 @@ public static Xpp3Dom build( XmlPullParser parser )

public static Xpp3Dom build( XmlPullParser parser, boolean trim )
throws XmlPullParserException, IOException
{
return build( parser, trim, null );
}

/**
* @since 3.2.0
*/
public static Xpp3Dom build( XmlPullParser parser, boolean trim, InputLocationBuilder locationBuilder )
throws XmlPullParserException, IOException
{
List<Xpp3Dom> elements = new ArrayList<Xpp3Dom>();

Expand All @@ -113,6 +140,11 @@ public static Xpp3Dom build( XmlPullParser parser, boolean trim )

Xpp3Dom childConfiguration = new Xpp3Dom( rawName );

if ( locationBuilder != null )
{
childConfiguration.setInputLocation( locationBuilder.toInputLocation( parser ) );
}

int depth = elements.size();

if ( depth > 0 )
Expand Down Expand Up @@ -194,4 +226,14 @@ else if ( eventType == XmlPullParser.END_TAG )

throw new IllegalStateException( "End of document found before returning to 0 depth" );
}

/**
* Input location builder interface, to be implemented to choose how to store data.
*
* @since 3.2.0
*/
public static interface InputLocationBuilder
{
Object toInputLocation( XmlPullParser parser );
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about 'getCurrentInputLocation'?

Should we explicitly state the expected behavior?

Is it better to return an GenericInputLocation, just as a placeholder interface? It will make code more typesafe and clearer IMHO

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'getCurrentInputLocation': since it is expected to create an object, I don't like 'get'

What do you mean by "state the expected behavior"?

Since the implementation is really something that will be specific to the user, and that the user will have to cast, I don't see the benefit of creating an empty interface

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'getCurrentInputLocation': since it is expected to create an object, I don't like 'get'

My concern is that the function takes a stateful object as parameter, so I image it will return a view of the current state of the parser.
the word 'to' seems to me like a 'conversion'.
Not so important to me, I am very new to this code.

What do you mean by "state the expected behavior"?

For instance, if the method is called twice is it expected to return two equivalent objects, the same object, or what ?

Object a =  toInputLocation( parser );
Object b =  toInputLocation( parser );

should b == a, b.quals(a) or it is not important ?

Since the implementation is really something that will be specific to the user, and that the user will have to cast, I don't see the benefit of creating an empty interface

Fine

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for input location, ordering is not important: really, the documentation to read is https://codehaus-plexus.github.io/modello/location-tracking.html

}
}
62 changes: 43 additions & 19 deletions src/main/java/org/codehaus/plexus/util/xml/Xpp3DomUtils.java
View file
Open in desktop
Expand Up @@ -19,6 +19,10 @@
import org.codehaus.plexus.util.xml.pull.XmlSerializer;

import java.io.IOException;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;

/** @author Jason van Zyl */
public class Xpp3DomUtils
Expand Down Expand Up @@ -71,24 +75,43 @@ public void writeToSerializer( String namespace, XmlSerializer serializer, Xpp3D
}

/**
* Merges one DOM into another, given a specific algorithm and possible override points for that algorithm. The
* algorithm is as follows: 1. if the recessive DOM is null, there is nothing to do...return. 2. Determine whether
* the dominant node will suppress the recessive one (flag=mergeSelf). A. retrieve the 'combine.self' attribute on
* the dominant node, and try to match against 'override'... if it matches 'override', then set mergeSelf ==
* false...the dominant node suppresses the recessive one completely. B. otherwise, use the default value for
* mergeSelf, which is true...this is the same as specifying 'combine.self' == 'merge' as an attribute of the
* dominant root node. 3. If mergeSelf == true A. if the dominant root node's value is empty, set it to the
* recessive root node's value B. For each attribute in the recessive root node which is not set in the dominant
* root node, set it. C. Determine whether children from the recessive DOM will be merged or appended to the
* dominant DOM as siblings (flag=mergeChildren). i. if childMergeOverride is set (non-null), use that value
* (true/false) ii. retrieve the 'combine.children' attribute on the dominant node, and try to match against
* 'append'...if it matches 'append', then set mergeChildren == false...the recessive children will be appended as
* siblings of the dominant children. iii. otherwise, use the default value for mergeChildren, which is true...this
* is the same as specifying 'combine.children' == 'merge' as an attribute on the dominant root node. D. Iterate
* through the recessive children, and: i. if 'combine.id' is set and there is a corresponding dominant child
* (matched by value of 'combine.id'), merge the two. ii. if mergeChildren == true and there is a corresponding
* dominant child (matched by element name), merge the two. iii. otherwise, add the recessive child as a new child
* on the dominant root node.
* Merges one DOM into another, given a specific algorithm and possible override points for that algorithm.<p>
* The algorithm is as follows:
* <ol>
* <li> if the recessive DOM is null, there is nothing to do... return.</li>
* <li> Determine whether the dominant node will suppress the recessive one (flag=mergeSelf).
* <ol type="A">
* <li> retrieve the 'combine.self' attribute on the dominant node, and try to match against 'override'...
* if it matches 'override', then set mergeSelf == false...the dominant node suppresses the recessive one
* completely.</li>
* <li> otherwise, use the default value for mergeSelf, which is true...this is the same as specifying
* 'combine.self' == 'merge' as an attribute of the dominant root node.</li>
* </ol></li>
* <li> If mergeSelf == true
* <ol type="A">
* <li> if the dominant root node's value is empty, set it to the recessive root node's value</li>
* <li> For each attribute in the recessive root node which is not set in the dominant root node, set it.</li>
* <li> Determine whether children from the recessive DOM will be merged or appended to the dominant DOM as
* siblings (flag=mergeChildren).
* <ol type="i">
* <li> if childMergeOverride is set (non-null), use that value (true/false)</li>
* <li> retrieve the 'combine.children' attribute on the dominant node, and try to match against
* 'append'...</li>
* <li> if it matches 'append', then set mergeChildren == false...the recessive children will be appended as
* siblings of the dominant children.</li>
* <li> otherwise, use the default value for mergeChildren, which is true...this is the same as specifying
* 'combine.children' == 'merge' as an attribute on the dominant root node.</li>
* </ol></li>
* <li> Iterate through the recessive children, and:
* <ol type="i">
* <li> if 'combine.id' is set and there is a corresponding dominant child (matched by value of 'combine.id'),
* merge the two.</li>
* <li> if mergeChildren == true and there is a corresponding dominant child (matched by element name),
* merge the two.</li>
* <li> otherwise, add the recessive child as a new child on the dominant root node.</li>
* </ol></li>
* </ol></li>
* </ol>
*/
private static void mergeIntoXpp3Dom( Xpp3Dom dominant, Xpp3Dom recessive, Boolean childMergeOverride )
{
Expand All @@ -109,9 +132,10 @@ private static void mergeIntoXpp3Dom( Xpp3Dom dominant, Xpp3Dom recessive, Boole

if ( mergeSelf )
{
if ( isEmpty( dominant.getValue() ) )
if ( isEmpty( dominant.getValue() ) && !isEmpty( recessive.getValue() ) )
{
dominant.setValue( recessive.getValue() );
dominant.setInputLocation( recessive.getInputLocation() );
}

String[] recessiveAttrs = recessive.getAttributeNames();
Expand Down
39 changes: 39 additions & 0 deletions src/test/java/org/codehaus/plexus/util/xml/Xpp3DomBuilderTest.java
View file
Open in desktop
Expand Up @@ -175,6 +175,35 @@ public void testEscapingInAttributes()
assertEquals( "Compare stringified DOMs", newString, s );
}

@Test
public void testInputLocationTracking()
throws IOException, XmlPullParserException
{
Xpp3DomBuilder.InputLocationBuilder ilb = new Xpp3DomBuilder.InputLocationBuilder() {
public Object toInputLocation( XmlPullParser parser )
{
return parser.getLineNumber(); // store only line number as a simple Integer
}

};
Xpp3Dom dom = Xpp3DomBuilder.build( new StringReader( createDomString() ), true, ilb );
Xpp3Dom expectedDom = createExpectedDom();
assertEquals( "root input location", expectedDom.getInputLocation(), dom.getInputLocation() );
for( int i = 0; i < dom.getChildCount(); i++ )
{
Xpp3Dom elt = dom.getChild( i );
Xpp3Dom expectedElt = expectedDom.getChild( i );
assertEquals( elt.getName() + " input location", expectedElt.getInputLocation(), elt.getInputLocation() );

if ( "el2".equals( elt.getName() ) )
{
Xpp3Dom el3 = elt.getChild( 0 );
Xpp3Dom expectedEl3 = expectedElt.getChild( 0 );
assertEquals( el3.getName() + " input location", expectedEl3.getInputLocation(), el3.getInputLocation() );
}
}
}

private static String getAttributeEncodedString()
{
StringBuilder domString = new StringBuilder();
Expand Down Expand Up @@ -237,23 +266,33 @@ private static String createDomString()

private static Xpp3Dom createExpectedDom()
{
int line = 1;
Xpp3Dom expectedDom = new Xpp3Dom( "root" );
expectedDom.setInputLocation( line );
Xpp3Dom el1 = new Xpp3Dom( "el1" );
el1.setInputLocation( ++line );
el1.setValue( "element1" );
expectedDom.addChild( el1 );
++line; // newline trimmed in Xpp3Dom but not in source
Xpp3Dom el2 = new Xpp3Dom( "el2" );
el2.setInputLocation( ++line );
el2.setAttribute( "att2", "attribute2\nnextline" );
expectedDom.addChild( el2 );
Xpp3Dom el3 = new Xpp3Dom( "el3" );
el3.setInputLocation( ++line );
el3.setAttribute( "att3", "attribute3" );
el3.setValue( "element3" );
el2.addChild( el3 );
++line;
Xpp3Dom el4 = new Xpp3Dom( "el4" );
el4.setInputLocation( ++line );
el4.setValue( "" );
expectedDom.addChild( el4 );
Xpp3Dom el5 = new Xpp3Dom( "el5" );
el5.setInputLocation( ++line );
expectedDom.addChild( el5 );
Xpp3Dom el6 = new Xpp3Dom( "el6" );
el6.setInputLocation( ++line );
el6.setAttribute( "xml:space", "preserve" );
el6.setValue( " do not trim " );
expectedDom.addChild( el6 );
Expand Down