arnaudroger / SimpleFlatMapper

Fast and Easy mapping from database and csv to POJO. A java micro ORM, lightweight alternative to iBatis and Hibernate. Fast Csv Parser and Csv Mapper
http://simpleflatmapper.org
MIT License
437 stars 76 forks source link

ASMUtils: Map<String, byte[]> not supported #662

Closed chriszwickerocteris closed 5 years ago

chriszwickerocteris commented 5 years ago

Mapping to a class with a field of type Map<String, byte[]> runs into a problem because the type is not properly extracted.

AsmUtils#parseTypes works correctly for a signature like "Ljava/util/Map<Ljava/lang/String;Ljava/lang/String;>;", of which Ljava/lang/String;Ljava/lang/String; gets passed to AsmUtils#parseTypes.

Unfortunately, because the method expects a trailing semicolon, it cannot deal with a signature like "Ljava/util/Map<Ljava/lang/String;[B>;", of which it gets Ljava/lang/String;[B (no trailing semicolon).

I'm not sure what the correct solution is, but from the limited samples I've looked at, I see two possible courses of action:

Maybe something like this?

--- a/sfm-reflect/src/main/java/org/simpleflatmapper/reflect/asm/AsmUtils.java
+++ b/sfm-reflect/src/main/java/org/simpleflatmapper/reflect/asm/AsmUtils.java
@@ -397,6 +397,8 @@ public static String toWrapperType(Type type) {

        int genericLevel = 0;
        int currentStart = 0;
+       int sigLength = sig.length();
+       int currentEnd = sigLength;
        for(int i = 0; i < sig.length(); i++) {
            char c = sig.charAt(i);
            switch(c) {
@@ -406,10 +408,15 @@ public static String toWrapperType(Type type) {
                    if (genericLevel == 0) {
                        types.add(toGenericType(sig.substring(currentStart, i), genericTypeNames, target));
                        currentStart = i + 1;
+                       currentEnd = i;
                    }
                    break;
            }
        }

+       if(currentEnd < sigLength - 1) {
+           types.add(toGenericType(sig.substring(currentStart), genericTypeNames, target));
+       }

        return types.toArray(EMPTY_TYPE_ARRAY);
    }
chriszwickerocteris commented 5 years ago

Should have mentioned I'm currently using 6.4.0 with jooq (org.simpleflatmapper:sfm-jooq:6.4.0)

arnaudroger commented 5 years ago

Thanks for the report, will see if there is a workaround.

arnaudroger commented 5 years ago

would you have the stacktrace for the errror?

chriszwickerocteris commented 5 years ago

Sure, here it is:

Message:
Index 1 out of bounds for length 1
Stack trace:
java.lang.ArrayIndexOutOfBoundsException: Index 1 out of bounds for length 1
    at org.simpleflatmapper.util.TypeHelper.getKeyValueTypeOfMap(TypeHelper.java:117)
    at org.simpleflatmapper.reflect.DefaultReflectionService.newMapMeta(DefaultReflectionService.java:222)
    at org.simpleflatmapper.reflect.DefaultReflectionService.newClassMeta(DefaultReflectionService.java:175)
    at org.simpleflatmapper.reflect.DefaultReflectionService.getClassMeta(DefaultReflectionService.java:145)
    at org.simpleflatmapper.reflect.meta.PropertyMeta.newPropertyClassMeta(PropertyMeta.java:53)
    at org.simpleflatmapper.reflect.meta.PropertyMeta.getPropertyClassMeta(PropertyMeta.java:46)
    at org.simpleflatmapper.reflect.meta.ObjectPropertyFinder.lookForSubProperty(ObjectPropertyFinder.java:145)
    at org.simpleflatmapper.reflect.meta.ObjectPropertyFinder.lookForConstructor(ObjectPropertyFinder.java:84)
    at org.simpleflatmapper.reflect.meta.ObjectPropertyFinder.lookForProperties(ObjectPropertyFinder.java:39)
    at org.simpleflatmapper.reflect.meta.PropertyFinder.findProperty(PropertyFinder.java:35)
    at org.simpleflatmapper.reflect.meta.PropertyFinder.findProperty(PropertyFinder.java:30)
    at org.simpleflatmapper.map.mapper.PropertyMappingsBuilder._addProperty(PropertyMappingsBuilder.java:96)
    at org.simpleflatmapper.map.mapper.PropertyMappingsBuilder.addProperty(PropertyMappingsBuilder.java:69)
    at org.simpleflatmapper.map.mapper.DefaultConstantSourceMapperBuilder.addMapping(DefaultConstantSourceMapperBuilder.java:159)
    at org.simpleflatmapper.map.mapper.SetRowMapperBuilderImpl.addMapping(SetRowMapperBuilderImpl.java:206)
    at org.simpleflatmapper.map.mapper.MapperBuilder.addMapping(MapperBuilder.java:142)
    at org.simpleflatmapper.map.mapper.MapperBuilder.addMapping(MapperBuilder.java:152)
    at org.simpleflatmapper.jdbc.JdbcMapperFactory$SetRowMapperFactory.newInstance(JdbcMapperFactory.java:344)
    at org.simpleflatmapper.jdbc.JdbcMapperFactory$SetRowMapperFactory.newInstance(JdbcMapperFactory.java:332)
    at org.simpleflatmapper.map.mapper.DynamicSetRowMapper.getMapper(DynamicSetRowMapper.java:104)
    at org.simpleflatmapper.map.mapper.DynamicSetRowMapper.getMapperFromSet(DynamicSetRowMapper.java:94)
    at org.simpleflatmapper.map.mapper.DynamicSetRowMapper.stream(DynamicSetRowMapper.java:70)
    at org.simpleflatmapper.jdbc.JdbcMapperFactory$DynamicJdbcSetRowMapper.stream(JdbcMapperFactory.java:285)

I'm calling mapper.stream(query.fetchResultSet());

my mapper configuration is

  protected final JdbcMapper<FileI18nResource> mapper =JdbcMapperFactory.newInstance()
    .useAsm(false)
    .ignorePropertyNotFound()
    .addKeys("id")
    .addColumnProperty("content_key", MapTypeProperty.KEY_VALUE)
    .newMapper(Foo.class)
  ;

and my entity has the following properties:

  private String id;
  private Map<String, byte[]> content;

  public Foo(final String id, final Map<String, byte[]> content) {
    this.id = id;
    this.content = content;
  }

and finally, I'm selecting

select 
  "root"."id", 
  "root"."content_key", 
  "root"."content_value"
from "root"
chriszwickerocteris commented 5 years ago

Thanks for looking into it!

arnaudroger commented 5 years ago

The type parser there is pretty buggy. Will rewrite it using the asm signature reader... hopefully won’t take too long.

arnaudroger commented 5 years ago

so I believe I;ve got a fix, I'll build it from the branch hopefully release tonight

arnaudroger commented 5 years ago

@chriszwickerocteris new version 6.7.1 is available in maven central, should fixed your issue, could you check it out?

chriszwickerocteris commented 5 years ago

@arnaudroger great, thank you! It does appear to work (so far, I've only run my tests, but I'm confident ;-)).

I've noticed you've left console outputs in the code (in edc1a84), wondered whether that was on purpose? Haven't looked at whether you do any preprocessing in the build...

arnaudroger commented 5 years ago

Was not will check that out, thanks for the help

chriszwickerocteris commented 5 years ago

Well, it's me who has to thank you!!

arnaudroger commented 5 years ago

the sout were remove in a later commit.