forked from Qortal/qortal
Fix API call GET /crosschain/trades (get completed trades) due to poorly performing SQL query.
Added "minimumTimestamp" param to same API call to allow fetching results for scenarios like: * completed trades since midnight * completed trades within last 24 hours Added corresponding tests for above API call, including checking call response times.
This commit is contained in:
parent
6c1b21da22
commit
f61e320230
@ -12,6 +12,7 @@ import io.swagger.v3.oas.annotations.tags.Tag;
|
|||||||
import java.math.BigDecimal;
|
import java.math.BigDecimal;
|
||||||
import java.util.ArrayList;
|
import java.util.ArrayList;
|
||||||
import java.util.Arrays;
|
import java.util.Arrays;
|
||||||
|
import java.util.Collections;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Random;
|
import java.util.Random;
|
||||||
import java.util.function.Function;
|
import java.util.function.Function;
|
||||||
@ -1223,6 +1224,10 @@ public class CrossChainResource {
|
|||||||
)
|
)
|
||||||
@ApiErrors({ApiError.INVALID_CRITERIA, ApiError.REPOSITORY_ISSUE})
|
@ApiErrors({ApiError.INVALID_CRITERIA, ApiError.REPOSITORY_ISSUE})
|
||||||
public List<CrossChainTradeSummary> getCompletedTrades(
|
public List<CrossChainTradeSummary> getCompletedTrades(
|
||||||
|
@Parameter(
|
||||||
|
description = "Only return trades that completed on/after this timestamp (milliseconds since epoch)",
|
||||||
|
example = "1597310000000"
|
||||||
|
) @QueryParam("minimumTimestamp") Long minimumTimestamp,
|
||||||
@Parameter( ref = "limit") @QueryParam("limit") Integer limit,
|
@Parameter( ref = "limit") @QueryParam("limit") Integer limit,
|
||||||
@Parameter( ref = "offset" ) @QueryParam("offset") Integer offset,
|
@Parameter( ref = "offset" ) @QueryParam("offset") Integer offset,
|
||||||
@Parameter( ref = "reverse" ) @QueryParam("reverse") Boolean reverse) {
|
@Parameter( ref = "reverse" ) @QueryParam("reverse") Boolean reverse) {
|
||||||
@ -1230,10 +1235,27 @@ public class CrossChainResource {
|
|||||||
if (limit != null && limit > 100)
|
if (limit != null && limit > 100)
|
||||||
throw ApiExceptionFactory.INSTANCE.createException(request, ApiError.INVALID_CRITERIA);
|
throw ApiExceptionFactory.INSTANCE.createException(request, ApiError.INVALID_CRITERIA);
|
||||||
|
|
||||||
|
// minimumTimestamp (if given) needs to be positive
|
||||||
|
if (minimumTimestamp != null && minimumTimestamp <= 0)
|
||||||
|
throw ApiExceptionFactory.INSTANCE.createException(request, ApiError.INVALID_CRITERIA);
|
||||||
|
|
||||||
final Boolean isFinished = Boolean.TRUE;
|
final Boolean isFinished = Boolean.TRUE;
|
||||||
final Integer minimumFinalHeight = null;
|
|
||||||
|
|
||||||
try (final Repository repository = RepositoryManager.getRepository()) {
|
try (final Repository repository = RepositoryManager.getRepository()) {
|
||||||
|
Integer minimumFinalHeight = null;
|
||||||
|
|
||||||
|
if (minimumTimestamp != null) {
|
||||||
|
minimumFinalHeight = repository.getBlockRepository().getHeightFromTimestamp(minimumTimestamp);
|
||||||
|
|
||||||
|
if (minimumFinalHeight == 0)
|
||||||
|
// We don't have any blocks since minimumTimestamp, let alone trades, so nothing to return
|
||||||
|
return Collections.emptyList();
|
||||||
|
|
||||||
|
// height returned from repository is for block BEFORE timestamp
|
||||||
|
// but we want trades AFTER timestamp so bump height accordingly
|
||||||
|
minimumFinalHeight++;
|
||||||
|
}
|
||||||
|
|
||||||
List<ATStateData> atStates = repository.getATRepository().getMatchingFinalATStates(BTCACCT.CODE_BYTES_HASH,
|
List<ATStateData> atStates = repository.getATRepository().getMatchingFinalATStates(BTCACCT.CODE_BYTES_HASH,
|
||||||
isFinished,
|
isFinished,
|
||||||
BTCACCT.MODE_BYTE_OFFSET, (long) BTCACCT.Mode.REDEEMED.value,
|
BTCACCT.MODE_BYTE_OFFSET, (long) BTCACCT.Mode.REDEEMED.value,
|
||||||
|
@ -301,7 +301,6 @@ public class HSQLDBATRepository implements ATRepository {
|
|||||||
+ "WHERE ATStates.AT_address = ATs.AT_address "
|
+ "WHERE ATStates.AT_address = ATs.AT_address "
|
||||||
+ "ORDER BY height DESC "
|
+ "ORDER BY height DESC "
|
||||||
+ "LIMIT 1 "
|
+ "LIMIT 1 "
|
||||||
+ "USING INDEX"
|
|
||||||
+ ") AS FinalATStates "
|
+ ") AS FinalATStates "
|
||||||
+ "WHERE code_hash = ? ");
|
+ "WHERE code_hash = ? ");
|
||||||
|
|
||||||
@ -309,7 +308,7 @@ public class HSQLDBATRepository implements ATRepository {
|
|||||||
bindParams.add(codeHash);
|
bindParams.add(codeHash);
|
||||||
|
|
||||||
if (isFinished != null) {
|
if (isFinished != null) {
|
||||||
sql.append("AND is_finished = ?");
|
sql.append("AND is_finished = ? ");
|
||||||
bindParams.add(isFinished);
|
bindParams.add(isFinished);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
38
src/test/java/org/qortal/test/api/CrossChainApiTests.java
Normal file
38
src/test/java/org/qortal/test/api/CrossChainApiTests.java
Normal file
@ -0,0 +1,38 @@
|
|||||||
|
package org.qortal.test.api;
|
||||||
|
|
||||||
|
import org.junit.Before;
|
||||||
|
import org.junit.Test;
|
||||||
|
import org.qortal.api.ApiError;
|
||||||
|
import org.qortal.api.resource.CrossChainResource;
|
||||||
|
import org.qortal.test.common.ApiCommon;
|
||||||
|
|
||||||
|
public class CrossChainApiTests extends ApiCommon {
|
||||||
|
|
||||||
|
private CrossChainResource crossChainResource;
|
||||||
|
|
||||||
|
@Before
|
||||||
|
public void buildResource() {
|
||||||
|
this.crossChainResource = (CrossChainResource) ApiCommon.buildResource(CrossChainResource.class);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testGetTradeOffers() {
|
||||||
|
assertNoApiError((limit, offset, reverse) -> this.crossChainResource.getTradeOffers(limit, offset, reverse));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testGetCompletedTrades() {
|
||||||
|
assertNoApiError((limit, offset, reverse) -> this.crossChainResource.getCompletedTrades(System.currentTimeMillis() /*minimumTimestamp*/, limit, offset, reverse));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testInvalidGetCompletedTrades() {
|
||||||
|
Integer limit = null;
|
||||||
|
Integer offset = null;
|
||||||
|
Boolean reverse = null;
|
||||||
|
|
||||||
|
assertApiError(ApiError.INVALID_CRITERIA, () -> this.crossChainResource.getCompletedTrades(-1L /*minimumTimestamp*/, limit, offset, reverse));
|
||||||
|
assertApiError(ApiError.INVALID_CRITERIA, () -> this.crossChainResource.getCompletedTrades(0L /*minimumTimestamp*/, limit, offset, reverse));
|
||||||
|
}
|
||||||
|
|
||||||
|
}
|
@ -1,16 +1,30 @@
|
|||||||
package org.qortal.test.common;
|
package org.qortal.test.common;
|
||||||
|
|
||||||
|
import static org.junit.Assert.*;
|
||||||
|
|
||||||
import java.lang.reflect.Field;
|
import java.lang.reflect.Field;
|
||||||
|
|
||||||
import org.eclipse.jetty.server.Request;
|
import org.eclipse.jetty.server.Request;
|
||||||
import org.junit.Before;
|
import org.junit.Before;
|
||||||
|
import org.qortal.api.ApiError;
|
||||||
|
import org.qortal.api.ApiException;
|
||||||
import org.qortal.repository.DataException;
|
import org.qortal.repository.DataException;
|
||||||
|
|
||||||
public class ApiCommon extends Common {
|
public class ApiCommon extends Common {
|
||||||
|
|
||||||
|
public static final long MAX_API_RESPONSE_PERIOD = 2_000L; // ms
|
||||||
|
|
||||||
public static final Boolean[] ALL_BOOLEAN_VALUES = new Boolean[] { null, true, false };
|
public static final Boolean[] ALL_BOOLEAN_VALUES = new Boolean[] { null, true, false };
|
||||||
public static final Boolean[] TF_BOOLEAN_VALUES = new Boolean[] { true, false };
|
public static final Boolean[] TF_BOOLEAN_VALUES = new Boolean[] { true, false };
|
||||||
|
|
||||||
|
public static final Integer[] SAMPLE_LIMIT_VALUES = new Integer[] { null, 0, 1, 20 };
|
||||||
|
public static final Integer[] SAMPLE_OFFSET_VALUES = new Integer[] { null, 0, 1, 5 };
|
||||||
|
|
||||||
|
@FunctionalInterface
|
||||||
|
public interface SlicedApiCall {
|
||||||
|
public abstract void call(Integer limit, Integer offset, Boolean reverse);
|
||||||
|
}
|
||||||
|
|
||||||
public static class FakeRequest extends Request {
|
public static class FakeRequest extends Request {
|
||||||
public FakeRequest() {
|
public FakeRequest() {
|
||||||
super(null, null);
|
super(null, null);
|
||||||
@ -48,4 +62,50 @@ public class ApiCommon extends Common {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public static void assertApiError(ApiError expectedApiError, Runnable apiCall, Long maxResponsePeriod) {
|
||||||
|
try {
|
||||||
|
long beforeTimestamp = System.currentTimeMillis();
|
||||||
|
apiCall.run();
|
||||||
|
|
||||||
|
if (maxResponsePeriod != null) {
|
||||||
|
long responsePeriod = System.currentTimeMillis() - beforeTimestamp;
|
||||||
|
if (responsePeriod > maxResponsePeriod)
|
||||||
|
fail(String.format("API call response period %d ms greater than max allowed (%d ms)", responsePeriod, maxResponsePeriod));
|
||||||
|
}
|
||||||
|
} catch (ApiException e) {
|
||||||
|
ApiError actualApiError = ApiError.fromCode(e.error);
|
||||||
|
assertEquals(expectedApiError, actualApiError);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
public static void assertApiError(ApiError expectedApiError, Runnable apiCall) {
|
||||||
|
assertApiError(expectedApiError, apiCall, MAX_API_RESPONSE_PERIOD);
|
||||||
|
}
|
||||||
|
|
||||||
|
public static void assertNoApiError(Runnable apiCall, Long maxResponsePeriod) {
|
||||||
|
try {
|
||||||
|
long beforeTimestamp = System.currentTimeMillis();
|
||||||
|
apiCall.run();
|
||||||
|
|
||||||
|
if (maxResponsePeriod != null) {
|
||||||
|
long responsePeriod = System.currentTimeMillis() - beforeTimestamp;
|
||||||
|
if (responsePeriod > maxResponsePeriod)
|
||||||
|
fail(String.format("API call response period %d ms greater than max allowed (%d ms)", responsePeriod, maxResponsePeriod));
|
||||||
|
}
|
||||||
|
} catch (ApiException e) {
|
||||||
|
fail("ApiException unexpected");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
public static void assertNoApiError(Runnable apiCall) {
|
||||||
|
assertNoApiError(apiCall, MAX_API_RESPONSE_PERIOD);
|
||||||
|
}
|
||||||
|
|
||||||
|
public static void assertNoApiError(SlicedApiCall apiCall) {
|
||||||
|
for (Integer limit : SAMPLE_LIMIT_VALUES)
|
||||||
|
for (Integer offset : SAMPLE_OFFSET_VALUES)
|
||||||
|
for (Boolean reverse : ALL_BOOLEAN_VALUES)
|
||||||
|
assertNoApiError(() -> apiCall.call(limit, offset, reverse));
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user