Maven plugin - merging listeners in web.xml is buggy

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Maven plugin - merging listeners in web.xml is buggy

Marcin Piela
In version 1.4.2 of the maven plugin there seems to be a bug when merging web.xml files containing multiple <listener> elements if the root (<web-app>) of web.xml contains the xmlns attrubute.
To reproduce the bug:
Create a junit test based on: 
org.codehaus.cargo.module.webapp.merge.WebXmlListenerMergerTest and just modify the file1 and file2 Strings in "testMergeListeners()" so that the <web-app> elements contain the xmlns attribute (e.g.  xmlns="http://java.sun.com/xml/ns/javaee"). The test should fail. 
Fix:
After some debugging I came to the conclusion that the tags array in org.codehaus.cargo.module.webapp.WebXml22Type is incomplete and adding 
new WebXmlTag(this, "listener-class"),
to it fixes the bug for me.

Marcin Piela
Reply | Threaded
Open this post in threaded view
|

Re: Maven plugin - merging listeners in web.xml is buggy

S. Ali Tokmen
Hi Marcin

Thanks for this e-mail, unfortunately many details are missing.

Could you send diff files / updated Java files for the testcase and also for the WebXml22Type?

Cheers

S. Ali Tokmen
http://ali.tokmen.com/

My IM, GSM, PGP and other contact details
are on http://contact.ali.tokmen.com
On 6/27/13 10:45 , Marcin Piela wrote:
In version 1.4.2 of the maven plugin there seems to be a bug when merging web.xml files containing multiple <listener> elements if the root (<web-app>) of web.xml contains the xmlns attrubute.
To reproduce the bug:
Create a junit test based on: 
org.codehaus.cargo.module.webapp.merge.WebXmlListenerMergerTest and just modify the file1 and file2 Strings in "testMergeListeners()" so that the <web-app> elements contain the xmlns attribute (e.g.  xmlns="http://java.sun.com/xml/ns/javaee"). The test should fail. 
Fix:
After some debugging I came to the conclusion that the tags array in org.codehaus.cargo.module.webapp.WebXml22Type is incomplete and adding 
new WebXmlTag(this, "listener-class"),
to it fixes the bug for me.

Marcin Piela

Reply | Threaded
Open this post in threaded view
|

Re: Maven plugin - merging listeners in web.xml is buggy

Marcin Piela
Hi,
attached you will find two .patch files which should help you reproduce the issue. 
Apply the WebXmlListenerMergerTest.patch first - this adds a failing test case.
Then applying the WebXml22Type.patch should fix the test.
I hope that helps.

Marcin


2013/6/28 S. Ali Tokmen <[hidden email]>
Hi Marcin

Thanks for this e-mail, unfortunately many details are missing.

Could you send diff files / updated Java files for the testcase and also for the WebXml22Type?

Cheers

S. Ali Tokmen
http://ali.tokmen.com/

My IM, GSM, PGP and other contact details
are on http://contact.ali.tokmen.com
On 6/27/13 10:45 , Marcin Piela wrote:
In version 1.4.2 of the maven plugin there seems to be a bug when merging web.xml files containing multiple <listener> elements if the root (<web-app>) of web.xml contains the xmlns attrubute.
To reproduce the bug:
Create a junit test based on: 
org.codehaus.cargo.module.webapp.merge.WebXmlListenerMergerTest and just modify the file1 and file2 Strings in "testMergeListeners()" so that the <web-app> elements contain the xmlns attribute (e.g.  xmlns="http://java.sun.com/xml/ns/javaee"). The test should fail. 
Fix:
After some debugging I came to the conclusion that the tags array in org.codehaus.cargo.module.webapp.WebXml22Type is incomplete and adding 
new WebXmlTag(this, "listener-class"),
to it fixes the bug for me.

Marcin Piela




---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email

WebXmlListenerMergerTest.patch (5K) Download Attachment
WebXml22Type.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Maven plugin - merging listeners in web.xml is buggy

S. Ali Tokmen
Hi Marcin

Thanks for the details.

I've opened https://jira.codehaus.org/browse/CARGO-1209 and also committed revision 3681 with your patch.

CARGO has a bi-monthly release cycle, so the corrected version (1.4.3) would probably go live mid-July.

Meanwhile, you can use version 1.4.3-SNAPSHOT using the instructions on http://cargo.codehaus.org/Maven2+Plugin+Installation#Maven2PluginInstallation-snapshots

Cheers

S. Ali Tokmen
http://ali.tokmen.com/

My IM, GSM, PGP and other contact details
are on http://contact.ali.tokmen.com
On 6/28/13 09:59 , Marcin Piela wrote:
Hi,
attached you will find two .patch files which should help you reproduce the issue. 
Apply the WebXmlListenerMergerTest.patch first - this adds a failing test case.
Then applying the WebXml22Type.patch should fix the test.
I hope that helps.

Marcin


2013/6/28 S. Ali Tokmen <[hidden email]>
Hi Marcin

Thanks for this e-mail, unfortunately many details are missing.

Could you send diff files / updated Java files for the testcase and also for the WebXml22Type?

Cheers

S. Ali Tokmen
http://ali.tokmen.com/

My IM, GSM, PGP and other contact details
are on http://contact.ali.tokmen.com
On 6/27/13 10:45 , Marcin Piela wrote:
In version 1.4.2 of the maven plugin there seems to be a bug when merging web.xml files containing multiple <listener> elements if the root (<web-app>) of web.xml contains the xmlns attrubute.
To reproduce the bug:
Create a junit test based on: 
org.codehaus.cargo.module.webapp.merge.WebXmlListenerMergerTest and just modify the file1 and file2 Strings in "testMergeListeners()" so that the <web-app> elements contain the xmlns attribute (e.g.  xmlns="http://java.sun.com/xml/ns/javaee"). The test should fail. 
Fix:
After some debugging I came to the conclusion that the tags array in org.codehaus.cargo.module.webapp.WebXml22Type is incomplete and adding 
new WebXmlTag(this, "listener-class"),
to it fixes the bug for me.

Marcin Piela




---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email