Encoding configuration is not used in every case

GoogleCodeExporter commented 9 years ago

What steps will reproduce the problem?
1. Windows default encoding is CP-1252. The file is encoded with UTF-8.
2. File contains special characters like (é,βούτυρος)
3. Configuration
    <configuration>
        <outputDir>target</outputDir>
        <file>data/_ra_test_data_generation_generated.xml</file>
        <encoding>UTF-8</encoding>                  
        <replacements>
            <replacement>
                <token>&amp;REPLACE_PERFORM_METHODS;</token>
                <valueFile>data/perform_methods.pnuts</valueFile>
            </replacement>
        </replacements>
    </configuration>

What is the expected output? What do you see instead?

The placeholder &REPLACE_PERFORM_METHODS is replaced with the content of the 
file data/perform_methods.pnuts without problems. But the special characters 
are written in CP-1252 encoding and not UTF-8.
I would expect that the output is in UTF-8 like it is configured.

Original issue reported on code.google.com by ManfredK...@gmail.com on 7 Sep 2012 at 12:08

Attachments:

GoogleCodeExporter commented 9 years ago

What version of the product are you using?
1.5.1-SNAPSHOT

Please provide any additional information below.

- FileUtils should be a class with only static methods (no context information 
stored in the class, except file endcoding)
- There are at least three different instances of FileUtils 
(OutputFilenameBuilde, Replacement, ReplaceMojo) created but only in one case 
(ReplaceMojo) the encoding is set.
- That causes a problem in my case that buildReplacements() uses a different 
file encoding as the configured one.

I attached a patch for my solution of the source tree and a zip archive with 
the source files. The unit test still have the fix. I have no idea how to fix 
the mocked FileUtils tests.

Original comment by ManfredK...@gmail.com on 7 Sep 2012 at 12:10

GoogleCodeExporter commented 9 years ago

Who calls FileUtils.readFile() before the first step in the execution() method 
of the MoJo?

Original comment by ManfredK...@gmail.com on 7 Sep 2012 at 3:15

GoogleCodeExporter commented 9 years ago

Thanks! I'll have a look when I get home from work tonight.

(Usually I get emailed by Google Project when issues are reported and I didnt 
this time, so I'm sorry it took me so long to respond).

Original comment by baker.st...@gmail.com on 11 Sep 2012 at 2:13

Changed state: Accepted

GoogleCodeExporter commented 9 years ago

I debug the code a little bit. I found that in my case 
Replacement.setValueFile(String) is called at least two times befor the 
execute() method is called of the Mojo. So the encoding is not set from the 
configuration at this point of time.

com.google.code.maven_replacer_plugin.Replacement.setValueFile:54
sun.reflect.NativeMethodAccessorImpl.invoke0:-2
sun.reflect.NativeMethodAccessorImpl.invoke:39
sun.reflect.DelegatingMethodAccessorImpl.invoke:25
java.lang.reflect.Method.invoke:597
org.codehaus.plexus.component.configurator.converters.ComponentValueSetter.setVa
lueUsingSetter:253
org.codehaus.plexus.component.configurator.converters.ComponentValueSetter.confi
gure:302
org.codehaus.plexus.component.configurator.converters.composite.ObjectWithFields
Converter.processConfiguration:161
org.codehaus.plexus.component.configurator.converters.composite.ObjectWithFields
Converter.fromConfiguration:89
org.codehaus.plexus.component.configurator.converters.composite.CollectionConver
ter.fromChildren:153
org.codehaus.plexus.component.configurator.converters.composite.CollectionConver
ter.fromConfiguration:112
org.codehaus.plexus.component.configurator.converters.ComponentValueSetter.confi
gure:289
org.codehaus.plexus.component.configurator.converters.composite.ObjectWithFields
Converter.processConfiguration:161
org.codehaus.plexus.component.configurator.BasicComponentConfigurator.configureC
omponent:56
org.apache.maven.plugin.internal.DefaultMavenPluginManager.populatePluginFields:
567
org.apache.maven.plugin.internal.DefaultMavenPluginManager.getConfiguredMojo:529
org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo:92
org.apache.maven.lifecycle.internal.MojoExecutor.execute:209
org.apache.maven.lifecycle.internal.MojoExecutor.execute:153
org.apache.maven.lifecycle.internal.MojoExecutor.execute:145
org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject:84
org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject:59
org.apache.maven.lifecycle.internal.LifecycleStarter.singleThreadedBuild:183
org.apache.maven.lifecycle.internal.LifecycleStarter.execute:161
org.apache.maven.DefaultMaven.doExecute:320
org.apache.maven.DefaultMaven.execute:156
org.apache.maven.cli.MavenCli.execute:537
org.apache.maven.cli.MavenCli.doMain:196
org.apache.maven.cli.MavenCli.main:141
sun.reflect.NativeMethodAccessorImpl.invoke0:-2
sun.reflect.NativeMethodAccessorImpl.invoke:39
sun.reflect.DelegatingMethodAccessorImpl.invoke:25
java.lang.reflect.Method.invoke:597
org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced:290
org.codehaus.plexus.classworlds.launcher.Launcher.launch:230
org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode:409
org.codehaus.plexus.classworlds.launcher.Launcher.main:352

[error] read file data/perform_methods.pnuts with encoding: null

Original comment by ManfredK...@gmail.com on 11 Sep 2012 at 5:50

GoogleCodeExporter commented 9 years ago

I did some refactoring on your code. The new implementation is better then my 
last one. But not perfect. I changed some pieces of the code and the unit 
tests. Please have a look.
Now the files are read the first time after the execute() method is called. So 
the configured encoding is used for reading and writting of the files.
Hint: Some of the unittests have still errors because of my changes. But i 
don't know how to fix it. How can i mock static methods of a class?

Original comment by ManfredK...@gmail.com on 11 Sep 2012 at 6:09

Attachments:

GoogleCodeExporter commented 9 years ago

Hi, you cant mock a static method with the test libraries included in this 
plugin. That would require a something like PowerMock. I am not really willing 
to include that, but I can just look into it tonight if I get time.

In the meantime, you can just skip tests and install the plugin manually so you 
can use your fixes.

Thanks for all the contributions, I'll see if I can include them tonight (or 
hopefully this week sometime). :)

Original comment by baker.st...@gmail.com on 11 Sep 2012 at 6:26

GoogleCodeExporter commented 9 years ago

Hi, I have made some changes on trunk. It's a bit different to your diff but I 
think it's a bit simpler.

Rather than making FileUtils all static, I have opted to enforce encoding on 
the reads and writes which are then passed down from the ReplacerMojo.
This gave me more control for testing and also handles any potential 
multithreading that may be induced by maven3 (although the plugin should NOT be 
multithreading since it cannot tell if the same file is being operated on by 
multiple instances).

Original comment by baker.st...@gmail.com on 12 Sep 2012 at 11:14

Changed state: Started

GoogleCodeExporter commented 9 years ago

I'd like you to take a look and hopefully test it.

Thanks again for all your help,
Steven

Original comment by baker.st...@gmail.com on 12 Sep 2012 at 11:17

GoogleCodeExporter commented 9 years ago

It doesn't work:

[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal 
com.google.code.maven-replacer-plugin:replacer:1.5.1-SNAPSHOT:replace (default) 
on project ftps-ta-dsx: Unable to parse
 configuration of mojo com.google.code.maven-replacer-plugin:replacer:1.5.1-SNAPSHOT:replace for parameter replacement: Cannot find setter, adder nor
field in com.google.code.maven_replacer_plugin.Replacement for 'valueFile' -> 
[Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal 
com.google.code.maven-replacer-plugin:replacer:1.5.1-SNAPSHOT:replace (
default) on project ftps-ta-dsx: Unable to parse configuration of mojo 
com.google.code.maven-replacer-plugin:replacer:1.5.1-SNAPSHOT:replace for param
eter replacement: Cannot find setter, adder nor field in 
com.google.code.maven_replacer_plugin.Replacement for 'valueFile'
        at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:221)
        at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:153)
        at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:145)
        at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:84)
        at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:59)
        at org.apache.maven.lifecycle.internal.LifecycleStarter.singleThreadedBuild(LifecycleStarter.java:183)
        at org.apache.maven.lifecycle.internal.LifecycleStarter.execute(LifecycleStarter.java:161)
        at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:320)
        at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:156)
        at org.apache.maven.cli.MavenCli.execute(MavenCli.java:537)
        at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:196)
        at org.apache.maven.cli.MavenCli.main(MavenCli.java:141)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
        at java.lang.reflect.Method.invoke(Method.java:597)
        at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:290)
        at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:230)
        at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:409)
        at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:352)
Caused by: org.apache.maven.plugin.PluginConfigurationException: Unable to 
parse configuration of mojo com.google.code.maven-replacer-plugin:replacer:
1.5.1-SNAPSHOT:replace for parameter replacement: Cannot find setter, adder nor 
field in com.google.code.maven_replacer_plugin.Replacement for 'valueF
ile'
        at org.apache.maven.plugin.internal.DefaultMavenPluginManager.populatePluginFields(DefaultMavenPluginManager.java:597)
        at org.apache.maven.plugin.internal.DefaultMavenPluginManager.getConfiguredMojo(DefaultMavenPluginManager.java:529)
        at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:92)
        at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:209)
        ... 19 more
Caused by: 
org.codehaus.plexus.component.configurator.ComponentConfigurationException: 
Cannot find setter, adder nor field in com.google.code.maven_re
placer_plugin.Replacement for 'valueFile'
        at org.codehaus.plexus.component.configurator.converters.ComponentValueSetter.<init>(ComponentValueSetter.java:95)
        at org.codehaus.plexus.component.configurator.converters.composite.ObjectWithFieldsConverter.processConfiguration(ObjectWithFieldsConverter.ja
va:158)
        at org.codehaus.plexus.component.configurator.converters.composite.ObjectWithFieldsConverter.fromConfiguration(ObjectWithFieldsConverter.java:
89)
        at org.codehaus.plexus.component.configurator.converters.composite.CollectionConverter.fromChildren(CollectionConverter.java:153)
        at org.codehaus.plexus.component.configurator.converters.composite.CollectionConverter.fromConfiguration(CollectionConverter.java:112)
        at org.codehaus.plexus.component.configurator.converters.ComponentValueSetter.configure(ComponentValueSetter.java:289)
        at org.codehaus.plexus.component.configurator.converters.composite.ObjectWithFieldsConverter.processConfiguration(ObjectWithFieldsConverter.ja
va:161)
        at org.codehaus.plexus.component.configurator.BasicComponentConfigurator.configureComponent(BasicComponentConfigurator.java:56)
        at org.apache.maven.plugin.internal.DefaultMavenPluginManager.populatePluginFields(DefaultMavenPluginManager.java:567)
        ... 22 more

Original comment by ManfredK...@gmail.com on 12 Sep 2012 at 12:11

GoogleCodeExporter commented 9 years ago

It's not a good idea to have multiple instances of FileUtils instead of using 
static methods. It is a stateless class. So what is the usage of an instance?
For the unit test you can write real files instead of mock the result.

Original comment by ManfredK...@gmail.com on 12 Sep 2012 at 12:18

GoogleCodeExporter commented 9 years ago

The main problem is that your read the valueFiles before the execution is 
started. Maven creates the Replacement instances by introspection and calls the 
method setValueFile(String). But at this point of time the configuration of the 
Mojo is not finished and the encoding is not set. 
I added some error logger to the code:

ReplacerMojo:

    public void execute() throws MojoExecutionException {
        getLog().error("start execution");

FileUtils:

    public void writeToFile(String outputFile, String content, String encoding) throws IOException {
        Log log = new SystemStreamLog();
        log.error("write file: '" + outputFile + "' with encoding: " + encoding);   

  and

    public String readFile(String file, String encoding) throws IOException {
        Log log = new SystemStreamLog();
        log.error("read file: '" + file + "' with encoding: " + encoding);

Replacement

    public void setValueFile(String valueFile) throws IOException {
        Log log = new SystemStreamLog();
        log.error("set value file: " + valueFile);

Here the result:

INFO] --- replacer:1.5.1-SNAPSHOT:replace (default) @ ftps-ta-dsx ---
error] set value file: data/perform_methods.pnuts
error] read file: 'data/perform_methods.pnuts' with encoding: null
error] set value file: data/perform_methods_marathon.pnuts
error] read file: 'data/perform_methods_marathon.pnuts' with encoding: null
error] set value file: data/call_perform_methods.pnuts
error] read file: 'data/call_perform_methods.pnuts' with encoding: null
ERROR] start execution
error] read file: '${project.build.directory 
}\..\\data\_mt_test_data_generation_generated.xml' with encoding: UTF-8
error] write file: '${project.build.directory 
}\..\\target\ra_test_data_generator\Subroutine\\_mt_test_data_generation_generat
ed.xml' with encoding:
TF-8
error] read file: '${project.build.directory 
}\..\\data\_ra_test_data_generation_generated.xml' with encoding: UTF-8
error] write file: '${project.build.directory 
}\..\\target\ra_test_data_generator\Subroutine\\_ra_test_data_generation_generat
ed.xml' with encoding:
TF-8
INFO] Replacement run on 2 files.
INFO]

Original comment by ManfredK...@gmail.com on 12 Sep 2012 at 12:40

GoogleCodeExporter commented 9 years ago

Sure, but I dont like having a non-final static field hanging around like that.

I'll keep working on it.

Original comment by baker.st...@gmail.com on 12 Sep 2012 at 1:03

GoogleCodeExporter commented 9 years ago

I have tested a different approach. Encoding should be a member (with setter 
and getter) in Replacement and configured first:

<replacement>
  <encoding>UTF-8</encoding>
  <token>&REPLACE_PERFORM_METHODS;</token>
  <valueFile>data/perform_methods.pnuts</valueFile>
</replacement>

Then the method can look like:

    public void setValueFile(String valueFile) throws IOException {
        if (valueFile != null) {
            setValue(fileUtils.readFile(valueFile, encoding));
        }
    }

and it works!!!

[error] set encoding: UTF-8
[error] set value file: data/perform_methods.pnuts
[error] read file: 'data/perform_methods.pnuts' with encoding: UTF-8
[error] set encoding: UTF-8
[error] set value file: data/perform_methods_marathon.pnuts
[error] read file: 'data/perform_methods_marathon.pnuts' with encoding: UTF-8
[error] set encoding: UTF-8
[error] set value file: data/call_perform_methods.pnuts
[error] read file: 'data/call_perform_methods.pnuts' with encoding: UTF-8
[ERROR] start execution
[error] read file: '${project.build.directory 
}\..\\data\_mt_test_data_generation_generated.xml' with encoding: UTF-8

Original comment by ManfredK...@gmail.com on 12 Sep 2012 at 1:05

GoogleCodeExporter commented 9 years ago

That's a good approach. I'll run with that :)

I have committed this (with some other bundled changes relating to it), could 
you test it again?

Original comment by baker.st...@gmail.com on 12 Sep 2012 at 1:15

GoogleCodeExporter commented 9 years ago

Sure.
When do you sleep? It is late in Australia.

Original comment by ManfredK...@gmail.com on 12 Sep 2012 at 1:21

GoogleCodeExporter commented 9 years ago

Haha, I'll being going to sleep soon :)

Original comment by baker.st...@gmail.com on 12 Sep 2012 at 1:21

GoogleCodeExporter commented 9 years ago

It works for me. Thank you. Well done!

Original comment by ManfredK...@gmail.com on 12 Sep 2012 at 1:29

GoogleCodeExporter commented 9 years ago

Great, I'll prepare a release sometime this week.

Original comment by baker.st...@gmail.com on 12 Sep 2012 at 1:30

Changed state: Fixed

GoogleCodeExporter commented 9 years ago

It doesn't work in a multimodule environment. Sorry.
The path expansion is too late for this solution.

Original comment by ManfredK...@gmail.com on 12 Sep 2012 at 1:40

GoogleCodeExporter commented 9 years ago

Oh okay, I will keep working on this then.

Original comment by baker.st...@gmail.com on 13 Sep 2012 at 12:32

Changed state: Started

GoogleCodeExporter commented 9 years ago

How about I combine the fix for Issue 68 with this. i.e. default the encoding 
field to the source encoding property.

Original comment by baker.st...@gmail.com on 14 Sep 2012 at 6:17

GoogleCodeExporter commented 9 years ago

That is a good idea.

Original comment by ManfredK...@gmail.com on 14 Sep 2012 at 7:00

GoogleCodeExporter commented 9 years ago

Okay, I have committed the fix for Issue 68 and hopefully this will work for 
multimodules now.
You can set the source encoding with:
    <properties>
        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
    </properties>

Original comment by baker.st...@gmail.com on 14 Sep 2012 at 7:08

GoogleCodeExporter commented 9 years ago

From my testing, it looks like the source encoding property along with the 
fixes made in this issue are the way to go.

I think I will release shortly.

Original comment by baker.st...@gmail.com on 16 Sep 2012 at 9:14

Changed state: Fixed

GoogleCodeExporter commented 9 years ago

[deleted comment]

GoogleCodeExporter commented 9 years ago

Released in 1.5.1.

Original comment by baker.st...@gmail.com on 18 Sep 2012 at 12:26

Changed state: Released

GoogleCodeExporter commented 9 years ago

[deleted comment]

GoogleCodeExporter commented 9 years ago

Hi when I use the <filesToInclude> or <includes> none of this works for the 
version 1.5.2. No errors, but it doesnt get replaced the token and value.

So when I downgrade the version to 1.5.1 giving the following error.

[ERROR] Failed to execute goal 
com.google.code.maven-replacer-plugin:replacer:1.5.1:replace (default) on 
project reservation-service-jar: File 
'/test/workspace/my-service/service-jar/null' does not exist -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal 
com.google.code.maven-replacer-plugin:replacer:1.5.1:replace (default) on 
project reservation-service-jar: File 
'/test/workspace/my-service/service-jar/null' does not exist

However, I have set the correct path but it doesnt take the full path property 
and giving the null.

Any Idea?

Original comment by kosal...@gmail.com on 16 Apr 2013 at 11:44

GoogleCodeExporter commented 9 years ago

The configuration seems to be corrupt. You use placeholder for the input files, 
dont't you?
I use 1.5.1 with following configuraion without problems:

<configuration>
  <outputDir>target\replaced\</outputDir>
  <encoding>UTF-8</encoding>
  <preserveDir>false</preserveDir>
  <includes>
    <include>data/input1.xml</include>
    <include>data/input2.xml</include>
  </includes>
  <replacements>
   <replacement>

....

Original comment by ManfredK...@gmail.com on 17 Apr 2013 at 5:53

DhanaGitHub / maven-replacer-plugin

Encoding configuration is not used in every case #70