Skip to content

Enum parameter to repository method not handled. #1837

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

Closed
mikereiche opened this issue Oct 5, 2023 · 9 comments · Fixed by #1846 or #1856
Closed

Enum parameter to repository method not handled. #1837

mikereiche opened this issue Oct 5, 2023 · 9 comments · Fixed by #1846 or #1856

Comments

@mikereiche
Copy link
Collaborator

mikereiche commented Oct 5, 2023

TL;DR the issue is because the couchbaseCustomConversions created in CouchbaseDataConfiguration does not have the Enum converters.

@Bean(name = BeanNames.COUCHBASE_CUSTOM_CONVERSIONS)
@ConditionalOnMissingBean(name = BeanNames.COUCHBASE_CUSTOM_CONVERSIONS)
CouchbaseCustomConversions couchbaseCustomConversions() {
	return new CouchbaseCustomConversions(Collections.emptyList());
}

From @Pharisaeus comment on #1069 .

@mikereiche I hit the same issue when using a custom @query and named @param as in:

@query("UPDATE #{#n1ql.collection} SET status = $newStatus")
Result changeStatus(@param("newStatus") Status newStatus);
gives:

com.couchbase.client.core.error.InvalidArgumentException: Unsupported type for JSON value: class a.b.c.Status
at com.couchbase.client.core.error.InvalidArgumentException.fromMessage(InvalidArgumentException.java:28) ~[core-io-2.4.10.jar:na]
at com.couchbase.client.java.json.JsonValue.coerce(JsonValue.java:94) ~[java-client-3.4.10.jar:na]
at com.couchbase.client.java.json.JsonObject.put(JsonObject.java:222) ~[java-client-3.4.10.jar:na]
at org.springframework.data.couchbase.repository.query.StringBasedN1qlQueryParser.putNamedValue(StringBasedN1qlQueryParser.java:575) ~[spring-data-couchbase-5.1.4.jar:5.1.4]
at org.springframework.data.couchbase.repository.query.StringBasedN1qlQueryParser.getNamedPlaceholderValues(StringBasedN1qlQueryParser.java:517) ~[spring-data-couchbase-5.1.4.jar:5.1.4]
at org.springframework.data.couchbase.repository.query.StringBasedN1qlQueryParser.getPlaceholderValues(StringBasedN1qlQueryParser.java:543) ~[spring-data-couchbase-5.1.4.jar:5.1.4]
at org.springframework.data.couchbase.core.query.StringQuery.toN1qlSelectString(StringQuery.java:83) ~[spring-data-couchbase-5.1.4.jar:5.1.4]
at org.springframework.data.couchbase.core.ReactiveFindByQueryOperationSupport$ReactiveFindByQuerySupport.assembleEntityQuery(ReactiveFindByQueryOperationSupport.java:281) ~[spring-data-couchbase-5.1.4.jar:5.1.4]
at org.springframework.data.couchbase.core.ReactiveFindByQueryOperationSupport$ReactiveFindByQuerySupport.all(ReactiveFindByQueryOperationSupport.java:182) ~[spring-data-couchbase-5.1.4.jar:5.1.4]
at org.springframework.data.couchbase.core.ExecutableFindByQueryOperationSupport$ExecutableFindByQuerySupport.all(ExecutableFindByQueryOperationSupport.java:95) ~[spring-data-couchbase-5.1.4.jar:5.1.4]
at org.springframework.data.couchbase.repository.query.AbstractCouchbaseQuery.lambda$getExecutionToWrap$1(AbstractCouchbaseQuery.java:124) ~[spring-data-couchbase-5.1.4.jar:5.1.4]
at org.springframework.data.couchbase.repository.query.CouchbaseQueryExecution$ResultProcessingExecution.execute(CouchbaseQueryExecution.java:84) ~[spring-data-couchbase-5.1.4.jar:5.1.4]
at org.springframework.data.couchbase.repository.query.AbstractCouchbaseQuery.doExecute(AbstractCouchbaseQuery.java:93) ~[spring-data-couchbase-5.1.4.jar:5.1.4]
at org.springframework.data.couchbase.repository.query.AbstractCouchbaseQueryBase.execute(AbstractCouchbaseQueryBase.java:132) ~[spring-data-couchbase-5.1.4.jar:5.1.4]
at org.springframework.data.couchbase.repository.query.AbstractCouchbaseQueryBase.execute(AbstractCouchbaseQueryBase.java:113) ~[spring-data-couchbase-5.1.4.jar:5.1.4]
I'm guessing it goes through a different code path in this scenario and the previous fix is not enough.

@mikereiche
Copy link
Collaborator Author

mikereiche commented Oct 11, 2023

@Pharisaeus I can't reproduce this. At line 508, the parameter is converted to a json type. Can you provide the definition of your Status?
https://github.com/spring-projects/spring-data-couchbase/blob/247164c6c2e13beb4423a9898bb28a6678dd6d79/src/main/java/org/springframework/data/couchbase/repository/query/StringBasedN1qlQueryParser.java#L507C1-L516C53

I tried:

List<User> findByFirstname(@Param("firstName")FirstName firstName );

List<User> findByFirstnameIn(@Param("firstNames")FirstName[] firstNames );

@Query("#{#n1ql.selectEntity} where #{#n1ql.filter} and (firstname = $firstName)")
List<User> queryByFirstnameNamedParameter(@Param("firstName")FirstName firstName );

@Query("#{#n1ql.selectEntity} where #{#n1ql.filter} and (firstname = $1)")
List<User> queryByFirstnamePositionalParameter(@Param("firstName")FirstName firstName );

enum FirstName {
	Dave,
	William
}

@Query("#{#n1ql.selectEntity} where #{#n1ql.filter} and (jsonNode.myNumber = $myNumber)")
List<User> queryByIntegerEnumNamed(@Param("myNumber")IntEnum myNumber );

@Query("#{#n1ql.selectEntity} where #{#n1ql.filter} and (jsonNode.myNumber = $1)")
List<User> queryByIntegerEnumPositional(@Param("myNumber")IntEnum myNumber );

enum IntEnum {
	One(1),
	Two(2),
	OneThousand(1000);
	Integer value;
	IntEnum(Integer i){
		value = i;
	}
	@JsonValue
	public Integer getValue(){
		return value;
	}
}

@mikereiche
Copy link
Collaborator Author

Please reopen if you have additional information.

mikereiche added a commit that referenced this issue Oct 11, 2023
Just use the converted value as-is (in case of boolean or Number) enum value.

Closes #1837.
mikereiche added a commit that referenced this issue Oct 11, 2023
Just use the converted value as-is (in case of boolean or Number) enum value.

Closes #1837.
mikereiche added a commit that referenced this issue Oct 11, 2023
Just use the converted value as-is (in case of boolean or Number) enum value.

Closes #1837.
mikereiche added a commit that referenced this issue Oct 11, 2023
Just use the converted value as-is (in case of boolean or Number) enum value.

Closes #1837.
@Pharisaeus
Copy link

@mikereiche see: https://github.com/Pharisaeus/couchbase-enum it's as much bare-bones as I could make it. There are 2 tests at https://github.com/Pharisaeus/couchbase-enum/blob/master/src/test/java/org/example/test/SomeTest.java#L20 and both crash with the enum issue. The tests are running via testcontainers so you should be able to simply clone the repo and run tests as long as you have docker daemon running.

@mikereiche
Copy link
Collaborator Author

mikereiche commented Oct 12, 2023

The issue is that the 'couchbaseConverter' does not have the. EnumToObject converter factory, so it just passes through the enum instead of converting it to a json-friendly object.

Object value = couchbaseConverter.convertForWriteIfNeeded(rawValue);

The EnumToObject converter is added in AbstractCouchbaseConfiguration.customConversions() and is usually obtained with a "config" class that extends it. But you don't have such a class. I need to figure out where the configuration comes from in your test.

public CustomConversions customConversions(CryptoManager cryptoManager, ObjectMapper objectMapper) {
	List<Object> newConverters = new ArrayList();
	// The following
	newConverters.add(new OtherConverters.EnumToObject(getObjectMapper()));
	newConverters.add(new IntegerToEnumConverterFactory(getObjectMapper()));
	newConverters.add(new StringToEnumConverterFactory(getObjectMapper()));
	newConverters.add(new BooleanToEnumConverterFactory(getObjectMapper()));

@mikereiche mikereiche reopened this Oct 12, 2023
@Pharisaeus
Copy link

I only do https://github.com/Pharisaeus/couchbase-enum/blob/master/src/main/java/org/example/configuration/BeansConfiguration.java#L8

@EnableCouchbaseRepositories(basePackageClasses = {MyCouchbaseRepository.class})

and nothing more, so the configuration has to be some default/coming from springboot

@mikereiche
Copy link
Collaborator Author

mikereiche commented Oct 12, 2023

Yes - I see where it comes from. I have to move the addition of converters into CouchbaseCustomConversions constructor.

Edit: This won't work -> For a work-around you can add a class that extends AbstractCouchbaseConfiguration.

@mikereiche
Copy link
Collaborator Author

mikereiche commented Oct 12, 2023

You'll need back-ticks around value because it is a reserved word.

@Query("UPDATE #{#n1ql.collection} SET status = $newStatus WHERE #{#n1ql.filter} AND `value` in $values #{#n1ql.returning}")

And make your BeansConfiguration.java like this:

 package org.example.configuration;

import com.couchbase.client.core.encryption.CryptoManager;
import com.couchbase.client.java.encryption.annotation.Encrypted;
import com.couchbase.client.java.encryption.databind.jackson.EncryptionModule;
import com.couchbase.client.java.json.JsonValueModule;
import com.fasterxml.jackson.annotation.JsonValue;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.example.repository.MyCouchbaseRepository;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.data.convert.CustomConversions;
import org.springframework.data.convert.PropertyValueConverterRegistrar;
import org.springframework.data.convert.SimplePropertyValueConversions;
import org.springframework.data.couchbase.config.BeanNames;
import org.springframework.data.couchbase.core.convert.BooleanToEnumConverterFactory;
import org.springframework.data.couchbase.core.convert.CouchbaseCustomConversions;
import org.springframework.data.couchbase.core.convert.CouchbasePropertyValueConverterFactory;
import org.springframework.data.couchbase.core.convert.CryptoConverter;
import org.springframework.data.couchbase.core.convert.IntegerToEnumConverterFactory;
import org.springframework.data.couchbase.core.convert.JsonValueConverter;
import org.springframework.data.couchbase.core.convert.OtherConverters;
import org.springframework.data.couchbase.core.convert.StringToEnumConverterFactory;
import org.springframework.data.couchbase.core.mapping.CouchbaseMappingContext;
import org.springframework.data.couchbase.repository.config.EnableCouchbaseRepositories;

import java.lang.annotation.Annotation;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

@Configuration
@EnableCouchbaseRepositories(basePackageClasses = {MyCouchbaseRepository.class})
public class BeansConfiguration {

  volatile ObjectMapper objectMapper;
  volatile CryptoManager cryptoManager = null;

  @Bean(name = BeanNames.COUCHBASE_CUSTOM_CONVERSIONS)
  public CustomConversions customConversions() {
    return customConversions(getCryptoManager(), getObjectMapper());
  }

  /**
   * Register custom Converters in a {@link CustomConversions} object if required. These {@link CustomConversions} will
   * be registered with the {@link #mappingCouchbaseConverter(CouchbaseMappingContext, CouchbaseCustomConversions)} )}
   * and {@link #couchbaseMappingContext(CustomConversions)}.
   *
   * @param cryptoManager
   * @return must not be {@literal null}.
   */
  public CustomConversions customConversions(CryptoManager cryptoManager, ObjectMapper objectMapper) {
    List<Object> newConverters = new ArrayList();
    // The following
    newConverters.add(new OtherConverters.EnumToObject(getObjectMapper()));
    newConverters.add(new IntegerToEnumConverterFactory(getObjectMapper()));
    newConverters.add(new StringToEnumConverterFactory(getObjectMapper()));
    newConverters.add(new BooleanToEnumConverterFactory(getObjectMapper()));
    CustomConversions customConversions = CouchbaseCustomConversions.create(configurationAdapter -> {
      SimplePropertyValueConversions valueConversions = new SimplePropertyValueConversions();
      valueConversions.setConverterFactory(
          new CouchbasePropertyValueConverterFactory(cryptoManager, annotationToConverterMap(), objectMapper));
      valueConversions.setValueConverterRegistry(new PropertyValueConverterRegistrar().buildRegistry());
      valueConversions.afterPropertiesSet(); // wraps the CouchbasePropertyValueConverterFactory with CachingPVCFactory
      configurationAdapter.setPropertyValueConversions(valueConversions);
      configurationAdapter.registerConverters(newConverters);
    });
    return customConversions;
  }

  Map<Class<? extends Annotation>, Class<?>> annotationToConverterMap() {
    Map<Class<? extends Annotation>, Class<?>> map = new HashMap();
    map.put(Encrypted.class, CryptoConverter.class);
    map.put(JsonValue.class, JsonValueConverter.class);
    return map;
  }

  final public ObjectMapper getObjectMapper() {
    if (objectMapper == null) {
      synchronized (this) {
        if (objectMapper == null) {
          objectMapper = couchbaseObjectMapper();
        }
      }
    }
    return objectMapper;
  }

  /**
   * Creates a {@link ObjectMapper} for the jsonSerializer of the ClusterEnvironment and spring-data-couchbase
   * jacksonTranslationService and also some converters (EnumToObject, StringToEnum, IntegerToEnum)
   *
   * @return ObjectMapper
   */
  protected ObjectMapper couchbaseObjectMapper() {
    ObjectMapper om = new ObjectMapper();
    om.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
    om.registerModule(new JsonValueModule());
    if (getCryptoManager() != null) {
      om.registerModule(new EncryptionModule(getCryptoManager()));
    }
    return om;
  }
  /**
   * cryptoManager can be null, so it cannot be a bean and then used as an arg for bean methods
   */
  private CryptoManager getCryptoManager() {
    if (cryptoManager == null) {
      synchronized (this) {
        if (cryptoManager == null) {
          cryptoManager = cryptoManager();
        }
      }
    }
    return cryptoManager;
  }

  protected CryptoManager cryptoManager() {
    return null;
  }

public static Map<Class<? extends Annotation>, Class<?>> annotationToConverterMap() {
    Map<Class<? extends Annotation>, Class<?>> map = new HashMap();
    map.put(Encrypted.class, CryptoConverter.class);
    map.put(JsonValue.class, JsonValueConverter.class);
    return map;
  }
}

@mikereiche
Copy link
Collaborator Author

I do plan on fixing this, but that should get you going for now.

@mikereiche
Copy link
Collaborator Author

mikereiche commented Oct 13, 2023

This is a little tidier

package org.example.configuration;

import org.example.repository.MyCouchbaseRepository;
import org.springframework.boot.autoconfigure.couchbase.CouchbaseProperties;
import org.springframework.boot.autoconfigure.data.couchbase.CouchbaseDataProperties;
import org.springframework.context.annotation.Configuration;
import org.springframework.data.couchbase.config.AbstractCouchbaseConfiguration;
import org.springframework.data.couchbase.repository.config.EnableCouchbaseRepositories;

@Configuration
@EnableCouchbaseRepositories(basePackageClasses = { MyCouchbaseRepository.class })
public class BeansConfiguration extends AbstractCouchbaseConfiguration {

	private final CouchbaseProperties properties;
	private final CouchbaseDataProperties dataProperties;

	BeansConfiguration(CouchbaseProperties properties, CouchbaseDataProperties dataProperties) {
		this.properties = properties;
		this.dataProperties = dataProperties;
	}

	@Override
	public String getConnectionString() {
		return properties.getConnectionString();
	}

	@Override
	public String getUserName() {
		return properties.getUsername();
	}

	@Override
	public String getPassword() {
		return properties.getPassword();
	}

	@Override
	public String getBucketName() {
		return dataProperties.getBucketName();
	}

}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants