2.0.x don't work about spring-data HOT 15 CLOSED

I also upgraded our spring demo. It now uses arangodb-spring-data 2.1.3 and spring-boot-starter-parent 2.0.1.RELEASE.

from spring-data.

@mpv1989 I created an issue for this: https://jira.spring.io/browse/DATACMNS-1323

Let's see what they say.

from spring-data.

Can you use the 2.0.0.RELEASE of spring-boot-starter-parent? With 2.0.0 it seems to work. Maybe they changed the behavior of converter handling. I will have a look at this.
P.S.: Please switch to the current arangodb-spring-data version 2.1.3.

from spring-data.

Yes. It looks like there is now the need of annotating Converters with @ReadingConverter otherwise they are used for writing. I committed a fix

from spring-data.

The changelog of the 2.0.7.RELEASE of Spring Data Commons says:
* DATACMNS-1294 - Consider types in java.time to be simple.

They did the following in SimpleTypeHolder#isSimpleType:

if (typeName.startsWith("java.lang") || type.getName().startsWith("java.time")) {
	return true;
}

Instead of adding the java.time classes to their default type array (the registration of the default types can be prevented by passing false to the constructor), they added the check for java.time directly to the isSimpleType method.

The hardcoding of this check is not optimal, as this means that every Spring Data store needs to support java.time classes out of the box. But since we are on Java 8, it may be ok.

The writing converters in TimeStringConverters are thus not used anymore. The workaround with @ReadingConverter does solve this issue.

@mpv1989 as java.time classes are now considered to be simple types, should we move the reading converters from TimeStringConverters to ArangoSimpleTypeConverters and remove the writing converters as they have no effect anymore?

from spring-data.

@christian-lechner the writing converters in TimeStringConverters are still used. I don't know whether we should remove them. They behave different for LocalDate and LocaeDateTime like in Spring Data core where they are converted in format yyyy-MM-dd'T'HH:mm:ss.SSS'Z'. Our converters behave like described in #36

from spring-data.

You are right, they are still used. What I meant was: they should have no effect, since they are now simple types they should be passed to the Java driver as they are.

But Spring registers the org.springframework.data.convert.Jsr310Converters by default which does a different conversion as you wrote. So we need our writing converters ;)

from spring-data.

@mpv1989 the ADDITIONAL_DESERIALIZABLE_TYPES from ArangoExtCursorIterator can now also be removed.

As all java.time classes are now simple types, they are handed to the Java driver when doing a query. But not all types are supported by the VPackJdk8Module. Maybe we should add them, at least the "important" ones like Duration and Period (LocalTime, OffsetTime)?

from spring-data.

@christian-lechner

the ADDITIONAL_DESERIALIZABLE_TYPES from ArangoExtCursorIterator can now also be removed.

I would leave them in for backward compatibility.

But not all types are supported by the VPackJdk8Module. Maybe we should add them, at least the "important" ones like Duration and Period (LocalTime, OffsetTime)?

Good point.

Speaking of ArangoCursor and Spring Data 2.0.7-RELEASE, there is a commit which removes the possibility to use ArangoCursor as a return type for repository methods. :-(

from spring-data.

I would leave them in for backward compatibility.

Or add them to ArangoSimpleTypes.

Speaking of ArangoCursor and Spring Data 2.0.7-RELEASE, there is a commit which removes the possibility to use ArangoCursor as a return type for repository methods. :-(

I see. The generic type argument of ArangoCursor is not unwrapped anymore. That's not that much of a problem, since we can override the QueryMethod#getReturnedObjectType() method in ArangoQueryMethod:

private final TypeInformation<?> returnType;

public ArangoQueryMethod(final Method method, final RepositoryMetadata metadata, final ProjectionFactory factory) {
	super(method, metadata, factory);
	this.method = method;
	this.returnType = ClassTypeInformation.from(metadata.getRepositoryInterface()).getReturnType(method);
}
// ...
@Override
public Class<?> getReturnedObjectType() {
	if (ArangoCursor.class.isAssignableFrom(method.getReturnType())) {
		return returnType.getRequiredComponentType().getType();
	} else {
		return super.getReturnedObjectType();
	}
}

I think this should do the trick?

from spring-data.

Ok this leads to a NPE. As the super constructor creates a ResultProcessor which uses the getReturnedObjectType() method. But the member var method of ArangoQueryMethod is not yet set.

from spring-data.

The only thing we can do for now is in ArangoQueryMethod:

TypeInformation<?> getReturnType() {
	return returnType;
}

And in AbstractArangoQuery#getResultClass():

if (ArangoCursor.class.isAssignableFrom(method.getReturnType().getType())) {
	return method.getReturnType().getRequiredComponentType().getType();
}

from spring-data.

Yes, I also see no other (not dirty) way.

from spring-data.

@mpv1989 we should continue in #47.

from spring-data.

@Jeffsyl01 I just released version 2.1.4 which is compatible with Spring 5.0.2.RELEASE and Spring Data 2.0.7.RELEASE

from spring-data.