Skip to content

[fix](open api) Handle metadata pagination boundaries #67816

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

Draft
morrySnow wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

[fix](open api) Handle metadata pagination boundaries #67816

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
10 changes: 4 additions & 6 deletions fe/fe-core/src/main/java/org/apache/doris/analysis/LimitElement.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

package org.apache.doris.analysis;

import org.apache.doris.nereids.util.Utils;

import java.util.List;

/**
Expand Down Expand Up @@ -79,12 +81,8 @@ public <T> List<T> applyTo(List<T> rows) {
int size = rows.size();
long begin = Math.min(Math.max(offset, 0L), size);
long end = size;
if (hasLimit()) {
end = begin + limit;
// A negative sum means the long addition itself overflowed.
if (end < 0 || end > size) {
end = size;
}
if (hasLimit() && !Utils.addOverflows(begin, limit)) {
end = Math.min(begin + limit, size);
}
return rows.subList((int) begin, (int) end);
}
Expand Down
44 changes: 2 additions & 42 deletions fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/MetaInfoAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import org.apache.doris.common.DdlException;
import org.apache.doris.common.FeConstants;
import org.apache.doris.common.MetaNotFoundException;
import org.apache.doris.common.Pair;
import org.apache.doris.common.UserException;
import org.apache.doris.common.proc.ProcNodeInterface;
import org.apache.doris.common.proc.ProcResult;
Expand All @@ -33,7 +32,6 @@
import org.apache.doris.datasource.InternalCatalog;
import org.apache.doris.httpv2.controller.BaseController.ActionAuthorizationInfo;
import org.apache.doris.httpv2.entity.ResponseEntityBuilder;
import org.apache.doris.httpv2.exception.BadRequestException;
import org.apache.doris.mysql.privilege.PrivPredicate;
import org.apache.doris.qe.ConnectContext;
import org.apache.doris.system.SystemInfoService;
Expand Down Expand Up @@ -64,8 +62,6 @@ public class MetaInfoAction extends RestBaseController {
private static final String NAMESPACES = "namespaces";
private static final String DATABASES = "databases";
private static final String TABLES = "tables";
private static final String PARAM_LIMIT = "limit";
private static final String PARAM_OFFSET = "offset";
private static final String PARAM_WITH_MV = "with_mv";


Expand Down Expand Up @@ -124,8 +120,7 @@ public Object getAllDatabases(
Collections.sort(visibleDbNames);

// handle limit offset
Pair<Integer, Integer> fromToIndex = getFromToIndex(request, visibleDbNames.size());
return ResponseEntityBuilder.ok(visibleDbNames.subList(fromToIndex.first, fromToIndex.second));
return ResponseEntityBuilder.ok(paginate(request, visibleDbNames));
}

/** Get all tables of a database
Expand Down Expand Up @@ -174,8 +169,7 @@ public Object getTables(
Collections.sort(tblNames);

// handle limit offset
Pair<Integer, Integer> fromToIndex = getFromToIndex(request, tblNames.size());
return ResponseEntityBuilder.ok(tblNames.subList(fromToIndex.first, fromToIndex.second));
return ResponseEntityBuilder.ok(paginate(request, tblNames));
}

/** Get schema of a table
Expand Down Expand Up @@ -309,38 +303,4 @@ private String convertIfNull(String val) {
return val.equals(FeConstants.null_string) ? null : val;
}

// get limit and offset from query parameter
// and return fromIndex and toIndex of a list
private Pair<Integer, Integer> getFromToIndex(HttpServletRequest request, int maxNum) {
String limitStr = request.getParameter(PARAM_LIMIT);
String offsetStr = request.getParameter(PARAM_OFFSET);

int offset = 0;
int limit = Integer.MAX_VALUE;
if (Strings.isNullOrEmpty(limitStr)) {
// limit not set
if (!Strings.isNullOrEmpty(offsetStr)) {
throw new BadRequestException("Param offset should be set with param limit");
}
} else {
// limit is set
limit = Integer.valueOf(limitStr);
if (limit < 0) {
throw new BadRequestException("Param limit should >= 0");
}

offset = 0;
if (!Strings.isNullOrEmpty(offsetStr)) {
offset = Integer.valueOf(offsetStr);
if (offset < 0) {
throw new BadRequestException("Param offset should >= 0");
}
}
}

if (maxNum <= 0) {
return Pair.of(0, 0);
}
return Pair.of(Math.min(offset, maxNum - 1), Math.min(limit + offset, maxNum));
}
}
37 changes: 37 additions & 0 deletions fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/RestBaseController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

package org.apache.doris.httpv2.rest;

import org.apache.doris.analysis.LimitElement;
import org.apache.doris.analysis.UserIdentity;
import org.apache.doris.catalog.Env;
import org.apache.doris.common.Config;
Expand All @@ -27,6 +28,7 @@
import org.apache.doris.common.util.NetUtils;
import org.apache.doris.httpv2.controller.BaseController;
import org.apache.doris.httpv2.entity.ResponseEntityBuilder;
import org.apache.doris.httpv2.exception.BadRequestException;
import org.apache.doris.httpv2.exception.UnauthorizedException;
import org.apache.doris.master.MetaHelper;
import org.apache.doris.qe.ConnectContext;
Expand Down Expand Up @@ -59,6 +61,7 @@
import java.net.URI;
import java.net.URISyntaxException;
import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;
import javax.annotation.Nullable;
import javax.net.ssl.HttpsURLConnection;
Expand All @@ -74,8 +77,42 @@ public class RestBaseController extends BaseController {
protected static final String TXN_OPERATION_KEY = "txn_operation";
protected static final String SINGLE_REPLICA_KEY = "single_replica";
protected static final String FORWARD_MASTER_UT_TEST = "forward_master_ut_test";
private static final String PARAM_LIMIT = "limit";
private static final String PARAM_OFFSET = "offset";
private static final Logger LOG = LogManager.getLogger(RestBaseController.class);

/**
* Apply the limit and offset query parameters to a list.
*/
protected <T> List<T> paginate(HttpServletRequest request, List<T> rows) {
String limitString = request.getParameter(PARAM_LIMIT);
String offsetString = request.getParameter(PARAM_OFFSET);

if (Strings.isNullOrEmpty(limitString)) {
if (!Strings.isNullOrEmpty(offsetString)) {
throw new BadRequestException("Param offset should be set with param limit");
}
return new LimitElement(0, -1).applyTo(rows);
}

long limit = parseNonNegativeLong(limitString, PARAM_LIMIT);
long offset = Strings.isNullOrEmpty(offsetString)
? 0 : parseNonNegativeLong(offsetString, PARAM_OFFSET);
return new LimitElement(offset, limit).applyTo(rows);
}

private long parseNonNegativeLong(String value, String parameterName) {
try {
long parsedValue = Long.parseLong(value);
if (parsedValue >= 0) {
return parsedValue;
}
} catch (NumberFormatException ignored) {
// Converted to a stable bad-request response below.
}
throw new BadRequestException("Param " + parameterName + " should be a non-negative integer");
}

public ActionAuthorizationInfo executeCheckPassword(HttpServletRequest request,
HttpServletResponse response) throws UnauthorizedException {
ActionAuthorizationInfo authInfo = getAuthorizationInfo(request);
Expand Down
48 changes: 3 additions & 45 deletions fe/fe-core/src/main/java/org/apache/doris/httpv2/restv2/MetaInfoActionV2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,11 @@
import org.apache.doris.catalog.TableIf;
import org.apache.doris.common.FeConstants;
import org.apache.doris.common.MetaNotFoundException;
import org.apache.doris.common.Pair;
import org.apache.doris.common.UserException;
import org.apache.doris.datasource.CatalogIf;
import org.apache.doris.datasource.InternalCatalog;
import org.apache.doris.httpv2.controller.BaseController.ActionAuthorizationInfo;
import org.apache.doris.httpv2.entity.ResponseEntityBuilder;
import org.apache.doris.httpv2.exception.BadRequestException;
import org.apache.doris.httpv2.rest.RestBaseController;
import org.apache.doris.mysql.privilege.PrivPredicate;
import org.apache.doris.qe.ConnectContext;
Expand Down Expand Up @@ -63,8 +61,6 @@ public class MetaInfoActionV2 extends RestBaseController {
private static final String NAMESPACES = "namespaces";
private static final String DATABASES = "databases";
private static final String TABLES = "tables";
private static final String PARAM_LIMIT = "limit";
private static final String PARAM_OFFSET = "offset";
private static final String PARAM_WITH_MV = "with_mv";

/**
Expand Down Expand Up @@ -98,8 +94,7 @@ public Object getAllCatalogs(
ctlsNames.add(0, InternalCatalog.INTERNAL_CATALOG_NAME);

// handle limit offset
Pair<Integer, Integer> fromToIndex = getFromToIndex(request, ctlsNames.size());
return ResponseEntityBuilder.ok(ctlsNames.subList(fromToIndex.first, fromToIndex.second));
return ResponseEntityBuilder.ok(paginate(request, ctlsNames));
}

/**
Expand Down Expand Up @@ -146,8 +141,7 @@ public Object getAllDatabases(
Collections.sort(filteredDbNames);

// handle limit offset
Pair<Integer, Integer> fromToIndex = getFromToIndex(request, filteredDbNames.size());
return ResponseEntityBuilder.ok(filteredDbNames.subList(fromToIndex.first, fromToIndex.second));
return ResponseEntityBuilder.ok(paginate(request, filteredDbNames));
}

/** Get all tables of a database
Expand Down Expand Up @@ -202,8 +196,7 @@ public Object getTables(
Collections.sort(tblNames);

// handle limit offset
Pair<Integer, Integer> fromToIndex = getFromToIndex(request, tblNames.size());
return ResponseEntityBuilder.ok(tblNames.subList(fromToIndex.first, fromToIndex.second));
return ResponseEntityBuilder.ok(paginate(request, tblNames));
}

/**
Expand Down Expand Up @@ -362,41 +355,6 @@ private String convertIfNull(String val) {
return val.equals(FeConstants.null_string) ? null : val;
}

// get limit and offset from query parameter
// and return fromIndex and toIndex of a list
private Pair<Integer, Integer> getFromToIndex(HttpServletRequest request, int maxNum) {
String limitStr = request.getParameter(PARAM_LIMIT);
String offsetStr = request.getParameter(PARAM_OFFSET);

int offset = 0;
int limit = Integer.MAX_VALUE;
if (Strings.isNullOrEmpty(limitStr)) {
// limit not set
if (!Strings.isNullOrEmpty(offsetStr)) {
throw new BadRequestException("Param offset should be set with param limit");
}
} else {
// limit is set
limit = Integer.valueOf(limitStr);
if (limit < 0) {
throw new BadRequestException("Param limit should >= 0");
}

offset = 0;
if (!Strings.isNullOrEmpty(offsetStr)) {
offset = Integer.valueOf(offsetStr);
if (offset < 0) {
throw new BadRequestException("Param offset should >= 0");
}
}
}

if (maxNum <= 0) {
return Pair.of(0, 0);
}
return Pair.of(Math.min(offset, maxNum - 1), Math.min(limit + offset, maxNum));
}

@Getter
@Setter
public static class TableSchemaInfo {
Expand Down
59 changes: 59 additions & 0 deletions fe/fe-core/src/test/java/org/apache/doris/httpv2/rest/RestBaseControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,21 @@

package org.apache.doris.httpv2.rest;

import org.apache.doris.httpv2.exception.BadRequestException;
import org.apache.doris.thrift.TNetworkAddress;

import jakarta.servlet.http.HttpServletRequest;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

import java.util.Arrays;
import java.util.List;

public class RestBaseControllerTest {

private static final List<Integer> ROWS = Arrays.asList(0, 1, 2);

@Test
public void testBuildRedirectUrlPreservesEncodedPath() {
// Keep the original encoded path unchanged when rebuilding the redirect URL.
Expand Down Expand Up @@ -68,6 +74,55 @@ public void testBuildRedirectUrlToBackendForcesHttpEvenWhenRequestIsHttps() {
Assertions.assertEquals("http://be-host:8040/api/db/tbl/_stream_load?k=v", redirectUrl);
}

@Test
public void testPaginateDefaultAndNormalRanges() {
assertPage(null, null, Arrays.asList(0, 1, 2));
assertPage("1", null, Arrays.asList(0));
assertPage("1", "1", Arrays.asList(1));
assertPage("0", "1", Arrays.asList());
assertPage("10", "1", Arrays.asList(1, 2));
}

@Test
public void testPaginateLargeValues() {
assertPage("1", Long.toString(Long.MAX_VALUE), Arrays.asList());
assertPage(Long.toString(Long.MAX_VALUE), "1", Arrays.asList(1, 2));
}

@Test
public void testPaginateOffsetRequiresLimit() {
BadRequestException exception = Assertions.assertThrows(BadRequestException.class,
() -> paginate(null, "1"));
Assertions.assertEquals("Param offset should be set with param limit", exception.getMessage());
}

@Test
public void testPaginateInvalidParameters() {
assertInvalid("-1", null, "Param limit should be a non-negative integer");
assertInvalid("not-a-number", null, "Param limit should be a non-negative integer");
assertInvalid("9223372036854775808", null, "Param limit should be a non-negative integer");
assertInvalid("1", "-1", "Param offset should be a non-negative integer");
assertInvalid("1", "not-a-number", "Param offset should be a non-negative integer");
assertInvalid("1", "9223372036854775808", "Param offset should be a non-negative integer");
}

private void assertPage(String limit, String offset, List<Integer> expected) {
Assertions.assertEquals(expected, paginate(limit, offset));
}

private void assertInvalid(String limit, String offset, String expectedMessage) {
BadRequestException exception = Assertions.assertThrows(BadRequestException.class,
() -> paginate(limit, offset));
Assertions.assertEquals(expectedMessage, exception.getMessage());
}

private List<Integer> paginate(String limit, String offset) {
HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
Mockito.when(request.getParameter("limit")).thenReturn(limit);
Mockito.when(request.getParameter("offset")).thenReturn(offset);
return new TestRestController().paginateForTest(request, ROWS);
}

// Expose the protected helper so the redirect URL can be verified directly.
private static class TestRestController extends RestBaseController {
private String buildRedirectUrlForTest(HttpServletRequest request, TNetworkAddress addr,
Expand All @@ -79,5 +134,9 @@ private String buildRedirectUrlToBackendForTest(HttpServletRequest request, TNet
String requestPath, String queryString) {
return buildRedirectUrlToBackend(request, addr, requestPath, queryString);
}

private <T> List<T> paginateForTest(HttpServletRequest request, List<T> rows) {
return paginate(request, rows);
}
}
}
Loading
Loading