search

paypal / butterfly

Application transformation tool
https://paypal.github.io/butterfly/
MIT License
47 stars 50 forks source link

Modify PomAddDependency to preserve file formatting and comments #92

Open fabiocarvalho777 opened 6 years ago

fabiocarvalho777 commented 6 years ago

Butterfly has one limitation when performing certain modifications to Maven pom files. Depending on the modification, it ends up reformatting the whole file, changing indentation and the order of the tags, besides also removing comments. That happens because Butterfly reads the file, parses it, builds a model in memory, applies the transformation, then writes it back to the same file.

When it writes the file with the transformation, it uses the model in memory as source, which means it looses entirely the original formatting and comments the file used to have. Some users don't like it because many times the transformation is very trivial, and would change just a few lines, but still the result is a file that looks entirely different.

The goal of this is issue is to focus on PomAddDependency, and modify it to perform isolated modifications, preserving formatting, order and comments (as explained earlier). That could be achieved using StAX.

This change in PomAddParent can be used as example.

StAX references

badalsarkar commented 3 years ago

Hi, I have started working on it.

fabiocarvalho777 commented 3 years ago

Cool, thank you very much!!

badalsarkar commented 3 years ago

Hi Fábio, I have done some work. This integration test is failing. 

java.lang.AssertionError: Baseline and transformed applications don't match, the contents of one file differ, as detailed below:

    Baseline application:    /home/badal/Documents/codeProject/butterfly/extensions-catalog/butterfly-springboot-extension/../../tests/transformed-baseline/echo-JavaEEToSpringBoot
    Transformed application: /tmp/butterfly-test1456204905109237410/echo-transformed-20210114172136901-d72b05af-601e-4082-8325-3749225838a0

    File:       /pom.xml
    At:         line 1, column 15
    Expected:   '"'
    Found:      '''
    at com.paypal.butterfly.test.FoldersComparison.failSingleDifferentFile(FoldersComparison.java:91)
    at com.paypal.butterfly.test.FoldersComparison.assertEquals(FoldersComparison.java:56)
    at com.paypal.butterfly.test.Assert.assertTransformation(Assert.java:149)
    at com.paypal.butterfly.test.Assert.assertTransformation(Assert.java:104)
    at com.paypal.butterfly.extensions.springboot.JavaEEToSpringBootIT.sampleAppRunTest(JavaEEToSpringBootIT.java:46)

Expected output is like

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://maven.apache.org/POM/4.0.0"
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>

but the actual output is like

<?xml version='1.0' encoding='UTF-8'?><project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
  <modelVersion>4.0.0</modelVersion>

This, I believe it is not related to my code. StAX is reading <?xml version="1.0" encoding="UTF-8"?> but writting <?xml version='1.0' encoding='UTF-8'?>. Also it is ignoring the \n at the end of DocumentStart element. I not sure if it is related to my system. I am still investigating.

Would you please take a look at the code? Or let me know if there is something I am missing.

I still need to add some tests and check the code.

Thanks. :smile:

fabiocarvalho777 commented 3 years ago

Hello Badal,

I took a look at your changes and I don't see any problem with it. Also, we can see here that PomAddDependency, the TO you are modifying as part of this issue, is being used in the transformation template used in that test. Having said that, I think it is just natural that the format of the top of the XML file would change. By the way, notice this difference is harmless. They are conceptually the same, but written differently. It is just a matter of preference between the Maven library we used to modify pom files in a DOM manner, versus StAX.

Having said that, please go ahead and change the baseline XML test file to match what StAX is producing, and the test should pass.

I hope this clarifies your questions. Let me know if not, or if you have other questions.

Thank you very much for working on this issue!

badalsarkar commented 3 years ago

Yes, this is clear to me. Working on it. :smile: