jmapper-framework / jmapper-core

Elegance, high performance and robustness all in one java bean mapper
http://jmapper-framework.github.io/jmapper-core
Apache License 2.0
227 stars 41 forks source link

Dollar character in replaceAll transformation #14

Closed spirion closed 9 years ago

spirion commented 9 years ago

In relation to issue #8 , the fix has replaced special characters in the regex part of the replaceAll and not the replacement string (second argument of the replaceAll method).

This is occurring in line 389 of the GeneralUtility class. Suggest the following fix:

text = text.replaceAll(Pattern.quote(prefix+var.getKey()), Pattern.quote(var.getValue()));

Thanks.

avurro commented 9 years ago

I have fixed it for conversions but not for basic mapping. I will work on this first of all.

spirion commented 9 years ago

Hi.

Thanks for your support here! I've done a couple of tests and I've discovered that my suggested solution doesn't work :(

I'll keep on plugging at it given the data I'm pushing through to see what I can come up with.

spirion commented 9 years ago

Hi. I think the following will work as a replacement for the current code:

text = text.replaceAll(Pattern.quote(prefix+var.getKey()), Matcher.quoteReplacement(var.getValue());

At least for the data I'm seeing go through the method.

avurro commented 9 years ago

You need to escape the $ with a double back slash, for example:

class Destination{               class Source{

  @JMap("str\\$");
  private String str$;               private String str$;

}                                }

special characters must be preceded by double backslash if you want use it as normal characters. Let me know if it works

spirion commented 9 years ago

Hi

Thanks for the info. I appreciate you spending time on this issue.

I'm not quite sure why the $ is being generated in the framework mappings as I don't have any inner or anonymous classes to map, but perhaps I've missed it. I'm not using $ in property names at all. I suspect this may be a result of, or at least a by-product of, the JAXB mapping process which generated the classes I'm mapping between (using XML, I can't use annotations in this case. I'll take a closer look tomorrow with fresh eyes in the morning and get back to you.

Thanks again!

spirion commented 9 years ago

Hi.

Apologies for the delay. I've done a little digging and it appears that the $ causing the issue in the mapping code generation is a result of the framework's generated code.

I have a conversion method in the XML configuration (package name redacted) :

<class name="com.*****.Duration">
    <global></global>
    <conversion name="conversion" from="unitOfTime" to="unitOfTime" type="STATIC">
        return com.*****.UnitOfTimeEnum.HOUR;
    </conversion>
</class>

The pertinent line of generated code is :

 obj35.setUnitOfTime(Duration$conversion());

That generates the following error :

08:31:36.368 [main] ERROR com.googlecode.jmapper.JMapper - IllegalArgumentException: Illegal group reference

If I apply the change to GeneralUtility I mentioned above (Matcher.quoteReplacement(var.getValue()), it _seems_ to works fine. I don't know if there are any side effects with that code alteration.

This looks like it may be outside of my control from the configuration point of view. Do you have any suggestions I could try?

Thanks.

avurro commented 9 years ago

i'm working on.. however i haven't touched the GeneralUtility method because before the class generation, i have applied to all methods a replacement of $. the code is in MapperGenarator.generateMapperClass, line 74:

// manages here all special characters
for (Method method : methods) 
    method.setBody(replace(method.getBody()));

private static String replace(String str){
    return str.replaceAll(Pattern.quote("$"), "\\$");
}

I'm investigating

spirion commented 9 years ago

Thanks for giving this your attention, I really appreciate it.

avurro commented 9 years ago

UnitOfTimeEnum.HOUR is a InnerClass, an instance of InnerClass or something that has to do with InnerClasses ? because Javassist does not support them

spirion commented 9 years ago

No its not, it's a stand alone enumeration.

avurro commented 9 years ago

this is the test i have done:

public class DestEnum {

    private EnumTestBean field;

    public EnumTestBean getField() {
        return field;
    }
    public void setField(EnumTestBean field) {
        this.field = field;
    }   
}

public class SrcEnum {

    private EnumTestBean field;

    public EnumTestBean getField() {
        return field;
    }

    public void setField(EnumTestBean field) {
        this.field = field;
    }

    public SrcEnum(EnumTestBean field) {
        super();
        this.field = field;
    }
}

public enum EnumTestBean {
    DESTINATION, SOURCE
}

xml configuration:

...
<class name="com.googlecode.jmapper.integrationtest.others.bean.DestEnum">
    <global></global>
    <conversion name="conversion" type="STATIC">
        return com.googlecode.jmapper.integrationtest.others.bean.EnumTestBean.DESTINATION;
    </conversion>
  </class>
...

It is finished successfully.
I can not recreate the error.. I need more information, can you write the code of the bean classes?

spirion commented 9 years ago

Hi.

I've created a project that shows the error:

https://github.com/spirion/jmapper-issue

Run the TestRunner class to view to issue. I've tried a number of configs, I can't get the issue to resolve. I've done the code change to GeneralUtilities locally which fixes this issue, no idea if it introduces any other problems though.

I hope this helps.

avurro commented 9 years ago

thank you, I'm working on.

avurro commented 9 years ago

I undestand the problem, the order in which the map is processed, however the issue is resolved as you have proposed. Sorry if I have not heard immediately

spirion commented 9 years ago

Great stuff! Thanks for your help, it's much appreciated. :)