Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance Converter #2486

Closed
HessTina-YuI opened this issue Mar 4, 2023 · 8 comments
Closed

Enhance Converter #2486

HessTina-YuI opened this issue Mar 4, 2023 · 8 comments
Labels
status: feedback-provided Feedback has been provided

Comments

@HessTina-YuI
Copy link

HessTina-YuI commented Mar 4, 2023

Question

In Spring Data Elasticsearch 4.0, removed jackson as the default json decompile.
So I need to write converter for my custom enum.

@Getter
@AllArgsConstructor(access = AccessLevel.PRIVATE)
public enum SexEnum {

    MALE("male", "the male"),
    FAMALE("famale", "the famale"),
    ;

    @JsonValue
    private final String code;

    private final String desc;

}

But I must write multiple Converter for my entity, they have the same function. However, I just can't get the class of the enumeration from converter.

@Document(indexName = "user")
@Data
public class User {

    private String userName;

    @ValueConverter(SexEnumConverter.class)
    private SexEnum sex;

    @ValueConverter(StatusEnumConverter.class)
    private StatusEnum status;

}

Because the read method can only get the String type.

public class SexEnumConverter implements PropertyValueConverter {

    @Override
    public Object write(Object value) {
        return null;
    }

    @Override
    public Object read(Object value) {
        return null;
    }

}

Solution Ideas

  1. Create interface add constructor to pass PersistentProperty or target class type, like org.springframework.data.elasticsearch.core.convert.AbstractPropertyValueConverter.
  2. Then add judgment condition initialization object in
    org.springframework.data.elasticsearch.core.mapping.SimpleElasticsearchPersistentProperty#initPropertyValueConverterFromAnnotation
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Mar 4, 2023
@sothawo
Copy link
Collaborator

sothawo commented Mar 5, 2023

I don't see why you need a custom converter for the enum at all. I just made a test with the following classes:

An enum with some custom fields

public enum Manufacturer {
	YAMAHA("Yamaha", "Japan"),
	TAKAMINE("Takamine", "Japan"),
	FENDER("Fender", "United States"),
	GUILD("Guild", "United States"),
	HOEFNER("Höfner", "Germany")
	;

	private final String displayName;
	private final String country;

	Manufacturer(String displayName, String country) {
		this.displayName = displayName;
		this.country = country;
	}

	public String getDisplayName() {
		return displayName;
	}

	public String getCountry() {
		return country;
	}
}

An entity class (record) using that enum:

@Document(indexName = "guitars")
public record Guitar(
		@Id String id,
		@Field(type = FieldType.Keyword)
		Manufacturer manufacturer,
		@Field(type = FieldType.Integer)
		Integer year) {
}

A repository with a method to get all guitars ordered by the year:

public interface GuitarRepository extends ElasticsearchRepository<Guitar, String> {
	List<Guitar> findAllByOrderByYearAsc();
}

And a controller that inserts some guitars, then fetches them again and displays them:

@RestController
@RequestMapping("/guitars")
public class GuitarController {

	private final GuitarRepository repository;

	public GuitarController(GuitarRepository repository) {
		this.repository = repository;
	}

	@GetMapping("/test")
	void test() {
		var takamine = new Guitar("1", Manufacturer.TAKAMINE, 2000);
		var guild = new Guitar("2", Manufacturer.GUILD, 1992);
		var fender = new Guitar("3", Manufacturer.FENDER, 2006);
		var yamaha = new Guitar("4", Manufacturer.YAMAHA, 2005);
		var hoefner = new Guitar("5", Manufacturer.HOEFNER, 1977);

		repository.saveAll(List.of(takamine, guild, fender, yamaha, hoefner));

		repository.findAllByOrderByYearAsc().forEach(guitar -> {
			System.out.println(String.format("In %d I bought a guitar manufactured by %s in %s",
					guitar.year(), guitar.manufacturer().getDisplayName(), guitar.manufacturer().getCountry()));
		});
	}
}

Running the application and calling /guitars/test results in the following output:

In 1977 I bought a guitar manufactured by Höfner in Germany
In 1992 I bought a guitar manufactured by Guild in United States
In 2000 I bought a guitar manufactured by Takamine in Japan
In 2005 I bought a guitar manufactured by Yamaha in Japan
In 2006 I bought a guitar manufactured by Fender in United States

So all works as it should.

Having a look at what is stored in Elasticsearch:

{
  "_shards": {
    "failed": 0,
    "skipped": 0,
    "successful": 1,
    "total": 1
  },
  "hits": {
    "hits": [
      {
        "_id": "1",
        "_index": "guitars",
        "_score": 1.0,
        "_source": {
          "_class": "com.sothawo.springdataelastictest.enums.Guitar",
          "id": "1",
          "manufacturer": "TAKAMINE",
          "year": 2000
        }
      },
      {
        "_id": "2",
        "_index": "guitars",
        "_score": 1.0,
        "_source": {
          "_class": "com.sothawo.springdataelastictest.enums.Guitar",
          "id": "2",
          "manufacturer": "GUILD",
          "year": 1992
        }
      },
      {
        "_id": "3",
        "_index": "guitars",
        "_score": 1.0,
        "_source": {
          "_class": "com.sothawo.springdataelastictest.enums.Guitar",
          "id": "3",
          "manufacturer": "FENDER",
          "year": 2006
        }
      },
      {
        "_id": "4",
        "_index": "guitars",
        "_score": 1.0,
        "_source": {
          "_class": "com.sothawo.springdataelastictest.enums.Guitar",
          "id": "4",
          "manufacturer": "YAMAHA",
          "year": 2005
        }
      },
      {
        "_id": "5",
        "_index": "guitars",
        "_score": 1.0,
        "_source": {
          "_class": "com.sothawo.springdataelastictest.enums.Guitar",
          "id": "5",
          "manufacturer": "HOEFNER",
          "year": 1977
        }
      }
    ],
    "max_score": 1.0,
    "total": {
      "relation": "eq",
      "value": 5
    }
  },
  "timed_out": false,
  "took": 3
}

Spring Data Elasticsearch maps the enum values to their enum name and has no problems converting them in either direction.

@sothawo sothawo added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Mar 5, 2023
@HessTina-YuI
Copy link
Author

HessTina-YuI commented Mar 5, 2023

Because, the default json framework for spring boot is jackson, and I use @JsonValue to serialize enum.
I know spring-data-elasticseach handles the enum by calling the name() method

public enum Manufacturer {
	YAMAHA("Yamaha", "Japan"),
	TAKAMINE("Takamine", "Japan"),
	FENDER("Fender", "United States"),
	GUILD("Guild", "United States"),
	HOEFNER("Höfner", "Germany")
	;

	@JsonValue
	private final String displayName;
	private final String country;

	Manufacturer(String displayName, String country) {
		this.displayName = displayName;
		this.country = country;
	}

	public String getDisplayName() {
		return displayName;
	}

	public String getCountry() {
		return country;
	}
}

When I use save() method, it's no problem. But I use update() method, I have to serialize my object with jackson, the resoult is displayName.

{
  "_shards": {
    "failed": 0,
    "skipped": 0,
    "successful": 1,
    "total": 1
  },
  "hits": {
    "hits": [
      {
        "_id": "1",
        "_index": "guitars",
        "_score": 1.0,
        "_source": {
          "_class": "com.sothawo.springdataelastictest.enums.Guitar",
          "id": "1",
          "manufacturer": "Takamine",
          "year": 2000
        }
      },
      {
        "_id": "2",
        "_index": "guitars",
        "_score": 1.0,
        "_source": {
          "_class": "com.sothawo.springdataelastictest.enums.Guitar",
          "id": "2",
          "manufacturer": "Guild",
          "year": 1992
        }
      },
      {
        "_id": "3",
        "_index": "guitars",
        "_score": 1.0,
        "_source": {
          "_class": "com.sothawo.springdataelastictest.enums.Guitar",
          "id": "3",
          "manufacturer": "Fender",
          "year": 2006
        }
      },
      {
        "_id": "4",
        "_index": "guitars",
        "_score": 1.0,
        "_source": {
          "_class": "com.sothawo.springdataelastictest.enums.Guitar",
          "id": "4",
          "manufacturer": "Yamaha",
          "year": 2005
        }
      },
      {
        "_id": "5",
        "_index": "guitars",
        "_score": 1.0,
        "_source": {
          "_class": "com.sothawo.springdataelastictest.enums.Guitar",
          "id": "5",
          "manufacturer": "Höfner",
          "year": 1977
        }
      }
    ],
    "max_score": 1.0,
    "total": {
      "relation": "eq",
      "value": 5
    }
  },
  "timed_out": false,
  "took": 3
}

In the end I could never fetch the object.

So, I need to create convertor for enum.
This is my EnumUilt

public class EnumUtil {

    private final static Map<Class<? extends Enum<?>>, Map<String, ? extends Enum<?>>> CLASS_STRING_ENUM =
            new ConcurrentHashMap<>();

    private final static Map<Class<? extends Enum<?>>, Map<? extends Enum<?>, String>> CLASS_ENUM_STRING =
            new ConcurrentHashMap<>();

    @SuppressWarnings("unchecked")
    public static <T extends Enum<T>> T match(Class<T> enumClass, String code) {
        if (Objects.isNull(enumClass) || StringUtils.isBlank(code)) {
            return null;
        }

        Map<String, ? extends Enum<?>> map = CLASS_STRING_ENUM.computeIfAbsent(enumClass,
                clazz -> {
                    Enum<?>[] enums = clazz.getEnumConstants();
                    Map<String, Enum<?>> nameMap = Arrays.stream(enums)
                            .collect(Collectors.toUnmodifiableMap(Enum::name, Function.identity()));

                    Field field = getField(clazz);
                    if (Objects.isNull(field)) {
                        return nameMap;
                    }

                    Map<String, Enum<?>> codeMap = Arrays.stream(enums)
                            .collect(Collectors.toUnmodifiableMap(e -> {
                                try {
                                    return (String) field.get(e);
                                } catch (IllegalAccessException ex) {
                                    throw new JsonException("enum handle error: " + enumClass.getName(), ex);
                                }
                            }, Function.identity()));

                    ImmutableMap.Builder<String, Enum<?>> builder = ImmutableMap.builder();
                    builder.putAll(nameMap)
                            .putAll(codeMap);
                    return builder.build();
                });

        return (T) map.getOrDefault(code, null);
    }

    public static <T extends Enum<T>> String getCode(Enum<T> enumObject) {
        if (Objects.isNull(enumObject)) {
            return null;
        }

        Class<T> enumClass = enumObject.getDeclaringClass();
        Map<? extends Enum<?>, String> map = CLASS_ENUM_STRING.computeIfAbsent(enumClass,
                clazz -> {
                    Field field = getField(clazz);

                    Enum<?>[] enums = clazz.getEnumConstants();
                    if (Objects.isNull(field)) {
                        return Arrays.stream(enums)
                                .collect(Collectors.toUnmodifiableMap(Function.identity(), Enum::name));
                    }


                    return Arrays.stream(enums)
                            .collect(Collectors.toUnmodifiableMap(Function.identity(),
                                    e -> {
                                        try {
                                            return (String) field.get(e);
                                        } catch (IllegalAccessException ex) {
                                            throw new JsonException("enum handle error: " + enumClass.getName(), ex);
                                        }
                                    })
                            );
                });

        return map.getOrDefault(enumObject, null);
    }

    private static Field getField(Class<? extends Enum<?>> clazz) {
        for (Field field : clazz.getDeclaredFields()) {
            JsonValue jsonValue = field.getAnnotation(JsonValue.class);
            if (Objects.nonNull(jsonValue)) {
                field.setAccessible(true);
                return field;
            }
        }
        return null;
    }

}

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Mar 5, 2023
@sothawo
Copy link
Collaborator

sothawo commented Mar 5, 2023

Because, the default json framework for spring boot is jackson, and I use @JsonValue to serialize enum.

Well, this is Spring Data Elasticsearch and not Spring Boot; what Boot uses is of no relevance here.

If you want to have a custom conversion using some other value than the enum's name, you have 2 possibilities

Use a general custom converter

(see reference docs):

import org.springframework.core.convert.converter.Converter;
import org.springframework.data.convert.WritingConverter;

@WritingConverter
public class ManufacturerWritingConverter implements Converter<Manufacturer, String> {
	@Override
	public String convert(Manufacturer source) {
		return source.getDisplayName();
	}
}
import org.springframework.core.convert.converter.Converter;
import org.springframework.data.convert.ReadingConverter;

@ReadingConverter
public class ManufacturerReadingConverter implements Converter<String, Manufacturer> {
	@Override
	public Manufacturer convert(String source) {
		return Manufacturer.of(source);
	}
}

register them in the configuration:

@Configuration
public class RestClientConfig extends ElasticsearchConfiguration {

	@Override
	public ClientConfiguration clientConfiguration() {
		
		return ClientConfiguration.builder() //
			.connectedTo("localhost:9200") //
			.build();
	}

	@Override
	public ElasticsearchCustomConversions elasticsearchCustomConversions() {
		Collection<Converter<?, ?>> converters = new ArrayList<>();
		converters.add(new ManufacturerWritingConverter());
		converters.add(new ManufacturerReadingConverter());
		return new ElasticsearchCustomConversions(converters);
	}
}

Use a PropertyValueConverter

mentioned in the reference docs here

import org.springframework.data.elasticsearch.core.mapping.PropertyValueConverter;

public class ManufacturerPropertyValueConverter implements PropertyValueConverter {
	@Override
	public Object write(Object value) {
		if (value instanceof Manufacturer m) {
			return m.getDisplayName();
		}
		return value;
	}

	@Override
	public Object read(Object value) {
		if (value instanceof String s) {
			var manufacturer = Manufacturer.of(s);
			if (manufacturer != null) {
				return manufacturer;
			}
		}
		return value;
	}
}

In this second case you have to change the definition in the entity class:

@Document(indexName = "guitars")
public record Guitar(
		@Id String id,
		@Field(type = FieldType.Keyword)
		@ValueConverter(ManufacturerPropertyValueConverter.class) // <-- 
		Manufacturer manufacturer,
		@Field(type = FieldType.Integer)
		Integer year) {
}

@HessTina-YuI
Copy link
Author

HessTina-YuI commented Mar 5, 2023

I know this way. But I have to create much conveter's class for my many enums.
Otherwise I have no way to know the type of the enum being converted in the ReadingConverter.
like

@Document(indexName = "user")
@Data
public class User {

    private String userName;

    @ValueConverter(SexEnumConverter.class)
    private SexEnum sex;

    @ValueConverter(StatusEnumConverter.class)
    private StatusEnum status;

}

Of course, if you would like to receive it, I can contribute the code.

@sothawo
Copy link
Collaborator

sothawo commented Mar 5, 2023

We definitely won't add code specific to Jackson to the Spring Data Elasticsearch code base.

The Spring Data base project has some property value converter support, and there it seems that during the conversion the type information for the property is available. (This was some time added after Spring Data Elasticsearch introduced property value converters and is more general).

To use this with Spring Data Elasticsearch, we'd need to to adapt the code to support these classes from spring-data-commons and then deprecated the existing code, I created #2488 for this.

@sothawo
Copy link
Collaborator

sothawo commented Mar 5, 2023

Addition: I found a solution that's faster to implement #2489.

The valueconverter changes to:

import org.springframework.data.elasticsearch.core.convert.AbstractPropertyValueConverter;
import org.springframework.data.mapping.PersistentProperty;

public class ManufacturerPropertyValueConverter extends AbstractPropertyValueConverter {
	public ManufacturerPropertyValueConverter(PersistentProperty<?> property) {
		super(property);
	}

	@Override
	public Object write(Object value) {
		if (value instanceof Manufacturer m) {
			return m.getDisplayName();
		}
		return value;
	}

	@Override
	public Object read(Object value) {
		if (value instanceof String s && Manufacturer.class.isAssignableFrom(getProperty().getType())) {
			var manufacturer = Manufacturer.of(s);
			if (manufacturer != null) {
				return manufacturer;
			}
		}
		return value;
	}
}

@HessTina-YuI
Copy link
Author

HessTina-YuI commented Mar 6, 2023

Thank you sir, this is the function I want. #2489 .
I know the spring-data-elasticsearch wants to handle serialization independently, I'm just saying that I need to be able to provide a place where I can get the class and I can be able to customize the richer Converter.

@HessTina-YuI
Copy link
Author

Unfortunately, I was hoping to finally have the opportunity to contribute code to spring-data-elasticsearch.
Of course, it's enough to solve my problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: feedback-provided Feedback has been provided
Projects
None yet
Development

No branches or pull requests

3 participants