Skip to content

Batch operation errors parsing #335

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
akudiyar merged 3 commits into from
Dec 29, 2022
Merged

Batch operation errors parsing #335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9d1bebf
Remove duplication in single result parser
akudiyar Dec 28, 2022
5102be4
Add crud batch errors handling to the single result parser
akudiyar Dec 28, 2022
215bc0f
Update CHANGELOG
akudiyar Dec 28, 2022
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
20 changes: 16 additions & 4 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,16 @@

## [Unreleased]

- Added `fields` option to ProxySpace ([#236](https://github.com/tarantool/cartridge-java/pull/236))
### Features
- Add `fields` option to ProxySpace for controlling the result tuple fields ([#236](https://github.com/tarantool/cartridge-java/pull/236))
- Parse metadata from crud response ([#272](https://github.com/tarantool/cartridge-java/pull/272))

### Bugfixes
- Add parsing of the batch operation errors ([#334](https://github.com/tarantool/cartridge-java/issues/334))

### Internal and API changes
- Move metadata parsing to separate converters ([#325](https://github.com/tarantool/cartridge-java/pull/325))
- Add simple to use TarantoolTupleResultMapper factory ([#321](https://github.com/tarantool/cartridge-java/pull/321))
- Parse metadata from crud response ([#272](https://github.com/tarantool/cartridge-java/pull/272))
- Use netty part dependencies instead of netty-all ([#295](https://github.com/tarantool/cartridge-java/issues/295))
- Refactor mappers and split `TarantoolResultConverter` ([#301](https://github.com/tarantool/cartridge-java/pull/301))
- **[breaking change]** `TarantoolResultConverter` was removed, use `DefaultArrayValueToTarantoolResultConverter` or `DefaultMapValueToTarantoolTupleResultConverter` instead ([#301](https://github.com/tarantool/cartridge-java/pull/301))
Expand All @@ -16,11 +22,14 @@
- **[breaking change]** `*MapperFactory` methods were renamed ([#301](https://github.com/tarantool/cartridge-java/pull/301))
- **[breaking change]** `*DefaultConverter` converters moved into `mappers.converters.value.defaults` package ([#301](https://github.com/tarantool/cartridge-java/pull/301))
- **[breaking change]** All converters from `mappers.converters.value.custom` moved into `mappers.converters.value` package ([#301](https://github.com/tarantool/cartridge-java/pull/301))
- Return jmh-generator-annprocess dependency
- Return back the `jmh-generator-annprocess` dependency

## [0.9.2] - 2022-11-15

### Features
- Adding default mapper for long arrays ([#290](https://github.com/tarantool/cartridge-java/pull/290))

### Internal and API changes
- Add dependency management ([#296](https://github.com/tarantool/cartridge-java/pull/296))
- Bump testcontainers-java-tarantool to 0.5.3 ([#296](https://github.com/tarantool/cartridge-java/pull/296))
- Bump slf4j-api to 2.0.3 ([#296](https://github.com/tarantool/cartridge-java/pull/296))
Expand All @@ -34,9 +43,12 @@

## [0.9.1] - 2022-10-13

### Features
- Enable Short to Integer converters ([#282](https://github.com/tarantool/cartridge-java/issues/282))

### Internal and API changes
- Changed TarantoolNullField class to singleton ([#195](https://github.com/tarantool/cartridge-java/pull/275))
- Bump netty to 4.1.78 ([#280](https://github.com/tarantool/cartridge-java/issues/280))
- Enable Short to Integer converters ([#282](https://github.com/tarantool/cartridge-java/issues/282))

### Security
- Bump jackson-databind to 2.14.1-rc1 ([#284](https://github.com/tarantool/cartridge-java/pull/284))
Expand Down
47 changes: 29 additions & 18 deletions src/main/java/io/tarantool/driver/core/SingleValueCallResultImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
import org.msgpack.value.ArrayValue;
import org.msgpack.value.Value;

import java.util.List;
import java.util.function.Function;

/**
* Basic {@link SingleValueCallResult} implementation. If the result array contains two values where the first is
* {@code null}, the second is treated as a formatted error or an error message.
Expand All @@ -20,34 +23,42 @@ public class SingleValueCallResultImpl<T> implements SingleValueCallResult<T> {
private final T value;

public SingleValueCallResultImpl(ArrayValue result, ValueConverter<Value, T> valueConverter) {
if (result == null) {
throw new TarantoolFunctionCallException("Function call result is null");
}
if (result.size() == 0 || result.size() == 1 && result.get(0).isNilValue()) {
// [nil] or []
value = null;
} else if (result.size() == 2 && (result.get(0).isNilValue() && !result.get(1).isNilValue())) {
// [nil, "Error msg..."] or [nil, {str="Error msg...", stack="..."}]
throw TarantoolErrorsParser.parse(result.get(1));
} else {
// [result]
value = valueConverter.fromValue(result.get(0));
}
value = parseResult(result, valueConverter::fromValue);
}

public SingleValueCallResultImpl(ArrayValue result, MessagePackValueMapper valueMapper) {
value = parseResult(result, valueMapper::fromValue);
}

private T parseResult(ArrayValue result, Function<Value, T> valueGetter) {
if (result == null) {
throw new TarantoolFunctionCallException("Function call result is null");
}
if (result.size() == 0 || result.size() == 1 && result.get(0).isNilValue()) {
int callResultSize = result.size();
Value resultValue = result.getOrNilValue(0);
Value errorsValue = result.getOrNilValue(1);
if (callResultSize == 0 || callResultSize == 1 && resultValue.isNilValue()) {
// [nil] or []
value = null;
} else if (result.size() == 2 && (result.get(0).isNilValue() && !result.get(1).isNilValue())) {
return null;
} else if (callResultSize == 2 && (resultValue.isNilValue() && !errorsValue.isNilValue())) {
// [nil, "Error msg..."] or [nil, {str="Error msg...", stack="..."}]
throw TarantoolErrorsParser.parse(result.get(1));
throw TarantoolErrorsParser.parse(errorsValue);
} else if (callResultSize == 2 && errorsValue.isArrayValue()) {
// [result, errors]
List<Value> errorsList = errorsValue.asArrayValue().list();
// We are not really interested in all errors, since the operation is already failed
if (!errorsList.isEmpty()) {
throw TarantoolErrorsParser.parse(errorsList.get(0));
}
throw new TarantoolFunctionCallException("Unexpected error format in the function call result");
} else if (callResultSize > 1 && !errorsValue.isNilValue()) {
throw new TarantoolFunctionCallException(
"Too many values in the function call result, " +
"expected \"[result]\", \"[result, errors]\" or \"[nil, error]\""
);
} else {
// [result]
value = valueMapper.fromValue(result.get(0));
return valueGetter.apply(resultValue);
}
}

Expand Down
52 changes: 35 additions & 17 deletions src/test/java/io/tarantool/driver/mappers/TarantoolCallResultMapperTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@
import io.tarantool.driver.api.TarantoolResult;
import io.tarantool.driver.api.tuple.TarantoolTuple;
import io.tarantool.driver.core.tuple.TarantoolTupleImpl;
import io.tarantool.driver.exceptions.TarantoolFunctionCallException;
import io.tarantool.driver.exceptions.TarantoolInternalException;
import io.tarantool.driver.exceptions.TarantoolTupleConversionException;
import io.tarantool.driver.mappers.factories.DefaultMessagePackMapperFactory;
import io.tarantool.driver.mappers.factories.ResultMapperFactoryFactoryImpl;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.msgpack.value.ArrayValue;
Expand Down Expand Up @@ -48,17 +48,9 @@ public static void setUp() {

@Test
void testSingleValueCallResultMapper() {
MessagePackMapper defaultMapper = DefaultMessagePackMapperFactory.getInstance().defaultComplexTypesMapper();
TarantoolTupleResultMapperFactory tarantoolTupleResultMapperFactory =
TarantoolTupleResultMapperFactoryImpl.getInstance();
CallResultMapper<TarantoolResult<TarantoolTuple>,
SingleValueCallResult<TarantoolResult<TarantoolTuple>>> mapper =
tarantoolTupleResultMapperFactory
.withSingleValueArrayToTarantoolTupleResultMapper(defaultMapper, null);

//[nil, message]
ArrayValue errorResult = ValueFactory.newArray(ValueFactory.newNil(), ValueFactory.newString("ERROR"));
assertThrows(TarantoolInternalException.class, () -> mapper.fromValue(errorResult));
assertThrows(TarantoolInternalException.class, () -> defaultResultMapper.fromValue(errorResult));

//[nil, {str=message, stack=stacktrace}]
MapValue error = ValueFactory.newMap(
Expand All @@ -68,17 +60,17 @@ void testSingleValueCallResultMapper() {
ValueFactory.newString("stacktrace")
);
ArrayValue errorResult1 = ValueFactory.newArray(ValueFactory.newNil(), error);
assertThrows(TarantoolInternalException.class, () -> mapper.fromValue(errorResult1));
assertThrows(TarantoolInternalException.class, () -> defaultResultMapper.fromValue(errorResult1));

//[[[],...]]
//[[[], ...]]
List<Object> nestedList1 = Arrays.asList("nested", "array", 1);
TarantoolTuple tupleOne = new TarantoolTupleImpl(Arrays.asList("abc", 1234, nestedList1), defaultMapper);
List<Object> nestedList2 = Arrays.asList("nested", "array", 2);
TarantoolTuple tupleTwo = new TarantoolTupleImpl(Arrays.asList("def", 5678, nestedList2), defaultMapper);
ArrayValue testTuples = ValueFactory.newArray(
tupleOne.toMessagePackValue(defaultMapper), tupleTwo.toMessagePackValue(defaultMapper));
ArrayValue callResult = ValueFactory.newArray(testTuples);
SingleValueCallResult<TarantoolResult<TarantoolTuple>> result = mapper.fromValue(callResult);
SingleValueCallResult<TarantoolResult<TarantoolTuple>> result = defaultResultMapper.fromValue(callResult);
TarantoolResult<TarantoolTuple> tuples = result.value();

assertEquals(2, tuples.size());
Expand All @@ -88,21 +80,47 @@ void testSingleValueCallResultMapper() {
assertEquals("def", tuples.get(1).getString(0));
assertEquals(5678, tuples.get(1).getInteger(1));
assertEquals(nestedList2, tuples.get(1).getList(2));

//[[[], ...], [message]]
ArrayValue errorResult2 =
ValueFactory.newArray(testTuples, ValueFactory.newArray(ValueFactory.newString("ERROR")));
assertThrows(TarantoolInternalException.class, () -> defaultResultMapper.fromValue(errorResult2));

//[[[], ...], [{str=message, stack=stacktrace}, {str=message, stack=stacktrace}]]
MapValue error1 = ValueFactory.newMap(error.asMapValue().map());
ArrayValue errors = ValueFactory.newArray(error, error1);
ArrayValue errorResult3 = ValueFactory.newArray(testTuples, errors);
assertThrows(TarantoolInternalException.class, () -> defaultResultMapper.fromValue(errorResult3));
}

@Test
void testDefaultTarantoolTupleResponse_singleResultShouldThrowException() {
void testDefaultTarantoolTupleResponse_singleResultShouldThrowException_cornerCase() {
ArrayValue testTuples = ValueFactory.newArray(
tupleOne.toMessagePackValue(defaultMapper), tupleTwo.toMessagePackValue(defaultMapper));

// The second value is a MsgPack array, and it is parsed an error. However, the result format is
// actually wrong, so the exact error contents does not matter in this case.
assertThrows(TarantoolInternalException.class, () -> defaultResultMapper.fromValue(testTuples), "def");
}

@Test
void testDefaultTarantoolTupleResponse_singleResultShouldThrowException() {
ArrayValue testTuples = ValueFactory.newArray(tupleOne.toMessagePackValue(defaultMapper));

// Another corner case, tuple result mapper should not be used for this result format
assertThrows(TarantoolTupleConversionException.class, () -> defaultResultMapper.fromValue(testTuples));
}

@Test
void testDefaultTarantoolTupleResponse_singleResultShouldThrowException_tooManyValues() {
ArrayValue testTuples = ValueFactory.newArray(tupleOne.toMessagePackValue(defaultMapper),
tupleOne.toMessagePackValue(defaultMapper), tupleOne.toMessagePackValue(defaultMapper));

assertThrows(TarantoolFunctionCallException.class, () -> defaultResultMapper.fromValue(testTuples));
}

@Test
void testMultiValueCallResultMapper() {
MessagePackMapper defaultMapper = DefaultMessagePackMapperFactory.getInstance().defaultComplexTypesMapper();
TarantoolTupleResultMapperFactory tarantoolTupleResultMapperFactory =
TarantoolTupleResultMapperFactoryImpl.getInstance();
CallResultMapper<TarantoolResult<TarantoolTuple>,
MultiValueCallResult<TarantoolTuple, TarantoolResult<TarantoolTuple>>> mapper =
tarantoolTupleResultMapperFactory
Expand Down