>From a functional standpoint it is working in our environment.  Using the
actual config reader to check the xml file is a more elegant solution than
using the DOM parse and element search.

Thanks for getting this resolved so quickly!

On Tue, May 5, 2015 at 11:46 AM, Romain Manni-Bucau <[email protected]>
wrote:

> https://issues.apache.org/jira/browse/TOMEE-1578 used a mix between your
> idea and mine, basically i load jaxb tree, check i need to add the
> Deployments then do a replace if I need to add it (avoid to break the whole
> formatting)
>
> happy to get a review/test from you if possible
>
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> <http://rmannibucau.wordpress.com> | Github <
> https://github.com/rmannibucau> |
> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Tomitriber
> <http://www.tomitribe.com>
>
> 2015-05-05 17:05 GMT+02:00 Adam Cornett <[email protected]>:
>
> > Sorry, by original I mean the one packaged in the original tomee zip
> file.
> >
> > On Tue, May 5, 2015 at 11:04 AM, Adam Cornett <[email protected]>
> > wrote:
> >
> > > That won't work since the original file contains the tag but it is
> > > commented out, so a simple string matching catches the value in the
> > comment
> > > and thinks its really there, that's why I opted for the 40-ton beast of
> > dom
> > > parsing.
> > >
> > > On Tue, May 5, 2015 at 11:00 AM, Romain Manni-Bucau <
> > [email protected]
> > > > wrote:
> > >
> > >> Ah ok, thought you were using tomee:build. Yes makes sense for
> > tomee:run.
> > >>
> > >> About the patch we can also be brutal and use:
> > >>
> > >> String content = IO.slurp(tomeeXml);
> > >> if (content.contains("<Deployments")) return; // user handles it
> > >> content = content.replace("</tomee>", "<Deployments ...." +
> > >> System.lineSeparator() + "</tomee>");
> > >> IO.write(tomeeXml, content);
> > >>
> > >> would work as well and be more readable.
> > >>
> > >> wdyt?
> > >>
> > >>
> > >>
> > >>
> > >> Romain Manni-Bucau
> > >> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > >> <http://rmannibucau.wordpress.com> | Github <
> > >> https://github.com/rmannibucau> |
> > >> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Tomitriber
> > >> <http://www.tomitribe.com>
> > >>
> > >> 2015-05-05 16:52 GMT+02:00 Adam Cornett <[email protected]>:
> > >>
> > >> > Perhaps I have not done a good job explaining how I'm using
> (abusing?)
> > >> the
> > >> > tomee plugin:
> > >> >
> > >> > I have a pom only project that uses the tomee maven plugin to
> > generate a
> > >> > tomee zip, adding in the conf, replacing openjpa with hibernate
> adding
> > >> > several other libraries, adding the setenv.(bat|sh) files with the
> > >> correct
> > >> > java settings, etc.  We use classifiers to create one zip per
> > >> environment
> > >> > (dev, qa, prod, etc)
> > >> > This zip file is attached to the project and published to our maven
> > >> repo.
> > >> >
> > >> > The plugin configuration is then set in our top level pom's
> > >> > pluginManagement and to use it in one of our projects we just have
> to
> > >> > include the groupId and artifactId.
> > >> >
> > >> > in pluginManagment:
> > >> > <plugin>
> > >> > <groupId>org.apache.openejb.maven</groupId>
> > >> > <artifactId>tomee-maven-plugin</artifactId>
> > >> > <version>${tomee.version}</version>
> > >> > <configuration>
> > >> > <tomeeGroupId>com.example</tomeeGroupId>
> > >> > <tomeeArtifactId>tomee</tomeeArtifactId>
> > >> > <tomeeClassifier>dev</tomeeClassifier>
> > >> > <tomeeVersion>${tomee.version}</tomeeVersion>
> > >> > </configuration>
> > >> > </plugin>
> > >> >
> > >> > in a project:
> > >> >
> > >> > <plugin>
> > >> > <groupId>org.apache.openejb.maven</groupId>
> > >> > <artifactId>tomee-maven-plugin</artifactId>
> > >> > </plugin>
> > >> >
> > >> > Thus each app can use mvn tomee:run for dev testing, and use use the
> > >> build
> > >> > goal to publish a tomee zip fully bundled up and ready to deploy to
> a
> > >> > server (we attach the zip to all of our EAR projects along with the
> > ear)
> > >> >
> > >> > This keeps the poms and source trees sane, previously we had to keep
> > >> > copies (using svn:externals, but still ugly) of the tomee config
> with
> > >> each
> > >> > ear.
> > >> >
> > >> > It also simplifies our Arquillian tests, since we are using such a
> > >> heavily
> > >> > modified version of TomEE we'd have to duplicate all of the changes
> in
> > >> the
> > >> > arquillian.xml, so a custom zip solves that problem too.
> > >> >
> > >> > Using a custom zip file I can keep up our internal tomee build and
> no
> > >> one
> > >> > else on the dev team has to worry about getting the right setup in
> > their
> > >> > checkout or pom to have it build and run correctly.
> > >> >
> > >> >
> > >> > The patch was attached to the original email is inline below and
> > >> > re-attached:
> > >> >
> > >> > diff --git
> > >> >
> > >>
> >
> a/maven/tomee-maven-plugin/src/main/java/org/apache/openejb/maven/plugin/AbstractTomEEMojo.java
> > >> >
> > >>
> >
> b/maven/tomee-maven-plugin/src/main/java/org/apache/openejb/maven/plugin/AbstractTomEEMojo.java
> > >> > index 6c8b412..86b13e8 100644
> > >> > ---
> > >> >
> > >>
> >
> a/maven/tomee-maven-plugin/src/main/java/org/apache/openejb/maven/plugin/AbstractTomEEMojo.java
> > >> > +++
> > >> >
> > >>
> >
> b/maven/tomee-maven-plugin/src/main/java/org/apache/openejb/maven/plugin/AbstractTomEEMojo.java
> > >> > @@ -78,6 +78,19 @@
> > >> >  import java.util.zip.ZipEntry;
> > >> >  import java.util.zip.ZipFile;
> > >> >
> > >> > +import javax.xml.parsers.DocumentBuilder;
> > >> > +import javax.xml.parsers.DocumentBuilderFactory;
> > >> > +import javax.xml.transform.OutputKeys;
> > >> > +import javax.xml.transform.Transformer;
> > >> > +import javax.xml.transform.TransformerFactory;
> > >> > +import javax.xml.transform.dom.DOMSource;
> > >> > +import javax.xml.transform.stream.StreamResult;
> > >> > +
> > >> > +import org.w3c.dom.Document;
> > >> > +import org.w3c.dom.Element;
> > >> > +import org.w3c.dom.NodeList;
> > >> > +
> > >> > +
> > >> >  import static org.apache.maven.artifact.Artifact.SCOPE_COMPILE;
> > >> >  import static
> > >> >
> > >>
> >
> org.apache.maven.artifact.repository.ArtifactRepositoryPolicy.CHECKSUM_POLICY_WARN;
> > >> >  import static
> > >> >
> > >>
> >
> org.apache.maven.artifact.repository.ArtifactRepositoryPolicy.UPDATE_POLICY_DAILY;
> > >> > @@ -1330,13 +1343,28 @@
> > >> >                  && (
> > >> >                  (apps != null && !apps.isEmpty())
> > >> >                          || (!"pom".equals(packaging) &&
> > >> > !"war".equals(packaging))))) { // webapps doesn't need apps folder
> in
> > >> tomee
> > >> > -            final FileWriter writer = new FileWriter(file);
> > >> > -            final String rootTag =
> > >> container.toLowerCase(Locale.ENGLISH);
> > >> > -            writer.write("<?xml version=\"1.0\"?>\n" +
> > >> > -                    "<" + rootTag + ">\n" +
> > >> > -                    "  <Deployments dir=\"apps\" />\n" +
> > >> > -                    "</" + rootTag + ">\n");
> > >> > -            writer.close();
> > >> > +            // check to see if there is already a Deployments tag
> in
> > >> the
> > >> > current file
> > >> > +            // if there isn't one, add it to the document
> > >> > +            try{
> > >> > +                DocumentBuilder builder =
> > >> > DocumentBuilderFactory.newInstance().newDocumentBuilder();
> > >> > +                Document doc = builder.parse(file);
> > >> > +
> > >> > +
> > if(doc.getElementsByTagName("Deployments").getLength()
> > >> ==
> > >> > 0){
> > >> > +                    Element deploymentsElement =
> > >> > doc.createElement("Deployments");
> > >> > +                    deploymentsElement.setAttribute("dir", "apps");
> > >> > +                    // get the 'tomee' or 'openejb' tag and add the
> > >> > deployments element
> > >> > +
> > >> >
> > >>
> >
> doc.getElementsByTagName(container.toLowerCase(Locale.ENGLISH)).item(0).appendChild(deploymentsElement);
> > >> > +
> > >> > +                    // now write the new xml file
> > >> > +                    Transformer transformer =
> > >> > TransformerFactory.newInstance().newTransformer();
> > >> > +
> transformer.setOutputProperty(OutputKeys.INDENT,
> > >> > "yes");
> > >> > +                    DOMSource source = new DOMSource(doc);
> > >> > +                    StreamResult fileOut = new StreamResult(file);
> > >> > +                    transformer.transform(source, fileOut);
> > >> > +                }
> > >> > +            }catch(Exception e){
> > >> > +                getLog().error("Exception updating " +
> > file.getName(),
> > >> e);
> > >> > +            }
> > >> >
> > >> >              final File appsFolder = new File(catalinaBase, "apps");
> > >> >              if (!appsFolder.exists() && !appsFolder.mkdirs()) {
> > >> >
> > >> >
> > >> >
> > >> >
> > >> >
> > >> >
> > >> > On Tue, May 5, 2015 at 10:29 AM, Romain Manni-Bucau <
> > >> [email protected]
> > >> > > wrote:
> > >> >
> > >> >> 2015-05-05 16:19 GMT+02:00 Adam Cornett <[email protected]>:
> > >> >>
> > >> >> > Like I said, there are many ways to work around this issue :-)
> > >> >> >
> > >> >> > I was just offering a change to the plugin to be more selective
> in
> > >> how
> > >> >> it
> > >> >> > changes the built in file to keep our build process as simple as
> > >> >> possible.
> > >> >> > I found it odd that the plugin was careful to honor custom
> changes
> > to
> > >> >> the
> > >> >> > server.xml file and only make selected changes to it, but it will
> > >> >> wholesale
> > >> >> > replace the tomee.xml file if you use 'ear' or 'ejb' packaging.
> > >> When it
> > >> >> > does this it also wipes out the documentation comments that ship
> > with
> > >> >> the
> > >> >> > default file.  The patch I sent keeps the original file's
> contents
> > >> and
> > >> >> > simply appends the deployments tag when needed if it is not
> already
> > >> >> > present, which is really the desired result of the method in
> > >> question.
> > >> >> >
> > >> >> >
> > >> >>
> > >> >> Well server.xml is another story. We fully control tomee.xml but
> not
> > >> >> server.xml and we try to not break tomcat update for this last one.
> > >> ALso
> > >> >> we
> > >> >> update this last one once we setup everything which can mean a user
> > can
> > >> >> have overriden the file which shouldn't be the case for tomee.xml.
> > >> >>
> > >> >> The easiest fix for your case is surely a flag
> 'dontTouchMyTomeeXml'
> > >> but
> > >> >> it
> > >> >> will not simplify your build. Alternative can be to support a
> > >> config.jar
> > >> >> but it is not simpler since you need to configure it again for all
> > >> >> distributions.
> > >> >>
> > >> >> Also what I dont get is why using tomee-maven-plugin if you start
> > from
> > >> an
> > >> >> existing tomee zip you provide. Doing it you loose the interest of
> > >> tomee
> > >> >> maven plugin build command which is to provide a standard layout.
> > >> >>
> > >> >> Just trying to see how to simplify the process without
> reimplementing
> > >> >> other
> > >> >> maven plugins
> > >> >>
> > >> >> Side note: not sure if gmail swallowed it but i didnt get any patch
> > >> >>
> > >> >>
> > >> >> > On Tue, May 5, 2015 at 10:07 AM, Romain Manni-Bucau <
> > >> >> [email protected]
> > >> >> > >
> > >> >> > wrote:
> > >> >> >
> > >> >> > > I see, why not executing maven dependency plugin before
> > >> >> > tomee-maven-plugin
> > >> >> > > and referencing the unpacked folder for the conf?
> > >> >> > >
> > >> >> > >
> > >> >> > > Romain Manni-Bucau
> > >> >> > > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > >> >> > > <http://rmannibucau.wordpress.com> | Github <
> > >> >> > > https://github.com/rmannibucau> |
> > >> >> > > LinkedIn <https://www.linkedin.com/in/rmannibucau> |
> Tomitriber
> > >> >> > > <http://www.tomitribe.com>
> > >> >> > >
> > >> >> > > 2015-05-05 16:01 GMT+02:00 Adam Cornett <
> [email protected]
> > >:
> > >> >> > >
> > >> >> > > > Since the conf is loaded into the zip file when we build it
> we
> > >> are
> > >> >> not
> > >> >> > > > re-supplying the conf each time we use the plugin.  Since all
> > of
> > >> our
> > >> >> > apps
> > >> >> > > > use a common conf we wanted it in one place instead of having
> > >> copies
> > >> >> > > > attached to each project.
> > >> >> > > >
> > >> >> > > > On Tue, May 5, 2015 at 9:57 AM, Romain Manni-Bucau <
> > >> >> > > [email protected]>
> > >> >> > > > wrote:
> > >> >> > > >
> > >> >> > > > > Hi
> > >> >> > > > >
> > >> >> > > > >
> > >> >> > > > > 2015-05-05 15:47 GMT+02:00 Adam Cornett <
> > >> [email protected]>:
> > >> >> > > > >
> > >> >> > > > > > We use TomEE in several applications, and to speed up
> > builds
> > >> and
> > >> >> > > > > > standardize our deployments, we use the Maven plugin to
> > >> create a
> > >> >> > > TomEE
> > >> >> > > > > zip
> > >> >> > > > > > already configured with our libraries and configuration
> > files
> > >> >> and
> > >> >> > > > publish
> > >> >> > > > > > it to our internal maven repo.
> > >> >> > > > > >
> > >> >> > > > > > We then use the TomEE maven plugin in each project to
> pull
> > in
> > >> >> this
> > >> >> > > > custom
> > >> >> > > > > > build and deploy our application into it.  However the
> > >> current
> > >> >> > maven
> > >> >> > > > > plugin
> > >> >> > > > > > will overwrite the conf/tomee.xml file that we have
> > packaged.
> > >> >> This
> > >> >> > > > > happens
> > >> >> > > > > > because the plugin is trying to be helpful by adding the
> > >> >> > > '<Deployments
> > >> >> > > > > > dir="apps" />' tag, which is not enabled in the file that
> > >> ships
> > >> >> > with
> > >> >> > > > the
> > >> >> > > > > > original zip, however it does this by writing out a whole
> > new
> > >> >> file
> > >> >> > > with
> > >> >> > > > > > this tag.
> > >> >> > > > > >
> > >> >> > > > > >
> > >> >> > > > > it does it during tomee unzip phase which is before the
> > >> >> > synchronization
> > >> >> > > > > with conf folder so your file should override the forced
> one.
> > >> >> > > > >
> > >> >> > > > > What do I miss?
> > >> >> > > > >
> > >> >> > > > >
> > >> >> > > > > > There are many ways to go about solving this issue, the
> > one I
> > >> >> came
> > >> >> > up
> > >> >> > > > > with
> > >> >> > > > > > is in the attached patch, developed against 2.0.  This
> > does a
> > >> >> more
> > >> >> > > > > surgical
> > >> >> > > > > > update of an existing tomee.xml file without touching
> over
> > >> any
> > >> >> > > existing
> > >> >> > > > > > changes.  If this needs further changes or refinement in
> > >> order
> > >> >> to
> > >> >> > be
> > >> >> > > > > > accepted into the project I'd be glad to continue to work
> > on
> > >> it
> > >> >> > based
> > >> >> > > > on
> > >> >> > > > > > feedback.
> > >> >> > > > > >
> > >> >> > > > > >
> > >> >> > > > > > - Adam Cornett
> > >> >> > > > > >
> > >> >> > > > > >
> > >> >> > > > > >
> > >> >> > > > > >
> > >> >> > > > > >
> > >> >> > > > >
> > >> >> > > >
> > >> >> > > >
> > >> >> > > >
> > >> >> > > > --
> > >> >> > > > Adam Cornett
> > >> >> > > > [email protected]
> > >> >> > > > (678) 296-1150
> > >> >> > > >
> > >> >> > >
> > >> >> >
> > >> >> >
> > >> >> >
> > >> >> > --
> > >> >> > Adam Cornett
> > >> >> > [email protected]
> > >> >> > (678) 296-1150
> > >> >> >
> > >> >>
> > >> >
> > >> >
> > >> >
> > >> > --
> > >> > Adam Cornett
> > >> > [email protected]
> > >> > (678) 296-1150
> > >> >
> > >>
> > >
> > >
> > >
> > > --
> > > Adam Cornett
> > > [email protected]
> > > (678) 296-1150
> > >
> >
> >
> >
> > --
> > Adam Cornett
> > [email protected]
> > (678) 296-1150
> >
>



-- 
Adam Cornett
[email protected]
(678) 296-1150

Reply via email to