tarantool / cartridge-springdata

Spring Data Tarantool
Other
18 stars 7 forks source link

TarantoolTemplate.call does not support result mapping when Lua function returns a 'map' instead of a 'tuple'. #45

Closed vrogach2020 closed 3 years ago

vrogach2020 commented 3 years ago

The fix for this issue may also deal with #32 and #28. Please check cases there.

Scenario:

  1. Setup a cluster with roles not using CRUD library.
  2. Create lua method:
    function test()
    return { 
    testString = "hello world",
    testInt = 123,
    }
    end
  3. Invoke TarantoolTemplate.call() to get result

public Class TestEntity {
  String testString ;
  Integer testInt;
}
// ...
TestEntity t = tarantoolTemplate.call("test");
vrogach2020 commented 3 years ago

@akudiyar please write some 'fix guidelines' for the issue.

wey1and commented 3 years ago

Error message:

io.tarantool.driver.exceptions.TarantoolClientException: The received tuple map has wrong format, expected {"metadata" : [...], "rows": [...]}, got {"field1": "value1"...}

akudiyar commented 3 years ago

The problem lies in that call is a callForList actually, and callForList executes TarantoolClient#call providing the result of type TarantoolResult<TarantoolTuple>. Then the mapper is applied, which takes TarantoolTuple as a source, an entity class, and converts the tuple to entity according to the space schema attached to the entity class via the @Tuple annotation. Recently the mapper received the ability to convert arbitrary maps into entities without schema information, it is used if the entity class has no @Tuple annotation and for the map fields of the tuple and nested maps.

We need to change the call and callForList implementation and add some new call methods so that the user will be able to specify which source is expected -- either a tuple (or list of tuples) or a single value.

I suggest that inside call we change the TarantoolClient#call to TarantoolClient#callForSingleResult and pass the corresponding custom result mapper to it, which will invoke TarantoolMappingConverter for converting the actual result into an entity or a collection of entities. The same is for callForList, but we also have a container mapper here. For distinguishing the source between tuples and maps, I suggest adding the following renaming of the API methods: call -> callForObject + callForTuple, callForList -> callForTupleList + callForObjectList. The parameter sets will repeat between both groups of methods. Seems that it is worth splitting off the CRUD and call operations from the TarantoolOperations interface (making it an aggregate) and splitting off the TarantoolTemplate into a base class with basic CRUD (TarantoolTemplateCRUD) and an extending class TarantoolTemplate with call methods implementation, for keeping the class files smaller.

Further, on the repository level, we will be able to select the corresponding call operation based on the annotations on entity classes, methods, and repository classes. I think that if an entity may be used both for mapping from tuples and from maps, it shouldn't have the @Tuple annotation, but this annotation will be present on a corresponding method in a repository. The alternative is using two entity classes, one with @Tuple annotation and one without. However, if an entity doesn't have a @Tuple annotation, it must be present on the corresponding repository for using the space schema for basic CRUD operations. If the @Tuple annotation is absent on both the entity and the repository, an exception must be thrown.

akudiyar commented 3 years ago

In connection with this task, we should also think about implementing the support for runtime schemas coming from tarantool/crud:

wey1and commented 3 years ago

During the discussions, the following points were made that need to be implemented:

These cases do not exclude each other, that means that all this can work in combination.

Required cases for @Tuple annotation:

vrogach2020 commented 3 years ago

@wey1and also we need to mention in RFC and in readme that custom repository methods return objects (not tuples) by default, but at the same time default repository methods (save, findAll etc) return tuples (not objects). Please correct me if I've misunderstood the implemented logic.