Skip to content

Expose ObjectMapper used by JsonSerializer #1132

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 5, 2021
Merged

Expose ObjectMapper used by JsonSerializer #1132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 47 additions & 4 deletions src/main/java/org/springframework/data/couchbase/config/AbstractCouchbaseConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@

package org.springframework.data.couchbase.config;

import static com.couchbase.client.java.ClusterOptions.*;
import static com.couchbase.client.java.ClusterOptions.clusterOptions;

import java.util.Collections;
import java.util.HashSet;
import java.util.Set;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.config.BeanDefinition;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.ClassPathScanningCandidateComponentProvider;
Expand All @@ -48,11 +49,19 @@
import org.springframework.util.StringUtils;

import com.couchbase.client.core.deps.com.fasterxml.jackson.databind.DeserializationFeature;
import com.couchbase.client.core.deps.com.fasterxml.jackson.databind.Module;
import com.couchbase.client.core.encryption.CryptoManager;
import com.couchbase.client.core.env.Authenticator;
import com.couchbase.client.core.env.PasswordAuthenticator;
import com.couchbase.client.core.error.CouchbaseException;
import com.couchbase.client.java.Cluster;
import com.couchbase.client.java.codec.JacksonJsonSerializer;
import com.couchbase.client.java.encryption.databind.jackson.EncryptionModule;
import com.couchbase.client.java.env.ClusterEnvironment;
import com.couchbase.client.java.json.JacksonTransformers;
import com.couchbase.client.java.json.JsonValueModule;
import com.couchbase.client.java.json.RepackagedJsonValueModule;
import com.fasterxml.jackson.databind.ObjectMapper;

/**
* Base class for Spring Data Couchbase configuration using JavaConfig.
Expand All @@ -65,12 +74,13 @@
@Configuration
public abstract class AbstractCouchbaseConfiguration {

@Autowired ObjectMapper couchbaseObjectMapper;

/**
* The connection string which allows the SDK to connect to the cluster.
* <p>
* Note that the connection string can take many forms, in its simplest it is just a single hostname
* like "127.0.0.1". Please refer to the couchbase Java SDK documentation for all the different
* possibilities and options.
* Note that the connection string can take many forms, in its simplest it is just a single hostname like "127.0.0.1".
* Please refer to the couchbase Java SDK documentation for all the different possibilities and options.
*/
public abstract String getConnectionString();

Expand Down Expand Up @@ -130,6 +140,10 @@ public Cluster couchbaseCluster(ClusterEnvironment couchbaseClusterEnvironment)
@Bean(destroyMethod = "shutdown")
public ClusterEnvironment couchbaseClusterEnvironment() {
ClusterEnvironment.Builder builder = ClusterEnvironment.builder();
if (!nonShadowedJacksonPresent()) {
throw new CouchbaseException("non-shadowed Jackson not present");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason we need to blow up if it is not present? (or just add the custom serializer if present?) - could log a warning instead if unlikely?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. In the SDK, when the non-shadowed jackson classes are not present, a DefaultJsonSerializer is created. DefaultJsonSerializer has no means to use an already-created mapper, or expose the mapper that it creates. I suppose this would only be an issue when the user wants to us couchbaseObjectMapper() and expects it to be the same one used by the serializer. However, David said the not having the non-shadowed classes in Spring is impossible. The alternative would be to issue a warning saying that the serializer was not using couchbaseObjectMapper() and continue.

return nonShadowedJacksonPresent()
? JacksonJsonSerializer.create(cryptoManager)
: DefaultJsonSerializer.create(cryptoManager);

}
builder.jsonSerializer(JacksonJsonSerializer.create(couchbaseObjectMapper));
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use the couchbaseObjectMapper from the new Bean.
Check for nonShadowedJacksonPresent() although it should never happen.

configureEnvironment(builder);
return builder.build();
}
Expand Down Expand Up @@ -273,6 +287,25 @@ public CouchbaseMappingContext couchbaseMappingContext(CustomConversions customC
return mappingContext;
}

/**
* Creates a {@link ObjectMapper} for the jsonSerializer of the ClusterEnvironment
*
* @throws Exception on Bean construction failure.
* @return ObjectMapper
*/

@Bean
public ObjectMapper couchbaseObjectMapper() {
ObjectMapper mapper = new ObjectMapper();
mapper.configure(com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
mapper.registerModule(new JsonValueModule());
CryptoManager cryptoManager = null;
if (cryptoManager != null) {
mapper.registerModule(new EncryptionModule(cryptoManager));
}
return mapper;
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the couchbaseObjectMapper that gets used and exposed. Created the same as in ClusterEnvironment.Builder.

/**
* Configure whether to automatically create indices for domain types by deriving the from the entity or not.
*/
Expand Down Expand Up @@ -327,4 +360,14 @@ protected FieldNamingStrategy fieldNamingStrategy() {
return abbreviateFieldNames() ? new CamelCaseAbbreviatingFieldNamingStrategy()
: PropertyNameFieldNamingStrategy.INSTANCE;
}

private boolean nonShadowedJacksonPresent() {
try {
JacksonJsonSerializer.preflightCheck();
return true;
} catch (Throwable t) {
return false;
}
}

}
4 changes: 2 additions & 2 deletions ...g/springframework/data/couchbase/repository/CouchbaseRepositoryQueryIntegrationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ void findByTypeAlias() {
vie = new Airport("airports::vie", "vie", "loww");
vie = airportRepository.save(vie);
List<Airport> airports = couchbaseTemplate.findByQuery(Airport.class)
.withConsistency(QueryScanConsistency.REQUEST_PLUS)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed a test that was sporadically failing.

.matching(new Query(QueryCriteria.where(N1QLExpression.x("_class")).is("airport"))).all();
assertFalse(airports.isEmpty(), "should have found aiport");
} finally {
Expand All @@ -198,7 +199,7 @@ void findByEnum() {
vie = new Airport("airports::vie", "vie", "loww");
vie = airportRepository.save(vie);
Airport airport2 = airportRepository.findByIata(Iata.vie);
assertNotNull(airport2, "should have found "+vie);
assertNotNull(airport2, "should have found " + vie);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

formatting. Nothing to see.

assertEquals(airport2.getId(), vie.getId());
} finally {
airportRepository.delete(vie);
Expand Down Expand Up @@ -397,7 +398,6 @@ void findBySimplePropertyAudited() {
}
}


private void sleep(int millis) {
try {
Thread.sleep(millis); // so they are executed out-of-order
Expand Down
14 changes: 8 additions & 6 deletions ...mework/data/couchbase/repository/ReactiveCouchbaseRepositoryKeyValueIntegrationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@

package org.springframework.data.couchbase.repository;

import static org.junit.jupiter.api.Assertions.*;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.util.List;
import java.util.Optional;
import java.util.UUID;

import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Bean;
Expand Down Expand Up @@ -64,13 +66,13 @@ void saveAndFindById() {
}

@Test
void findBySimplePropertyAudited() {
void findByIdAudited() {
Airport vie = null;
try {
vie = new Airport("airports::vie", "vie", "low2");
Airport saved = airportRepository.save(vie).block();
List<Airport> airports1 = airportRepository.findAllByIata("vie").collectList().block();
assertEquals(saved, airports1.get(0));
Airport airport1 = airportRepository.findById(saved.getId()).block();
assertEquals(airport1, saved);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was using a query which requires an index, but if you run this test by itself, there won't be an index. Switch to use kv.

assertEquals(saved.getCreatedBy(), "auditor"); // NaiveAuditorAware will provide this
} finally {
airportRepository.delete(vie).block();
Expand All @@ -79,7 +81,7 @@ void findBySimplePropertyAudited() {

@Configuration
@EnableReactiveCouchbaseRepositories("org.springframework.data.couchbase")
@EnableReactiveCouchbaseAuditing
@EnableReactiveCouchbaseAuditing
static class Config extends AbstractCouchbaseConfiguration {

@Override
Expand Down