Skip to content

Enforce REST query row limit as hard cap#18149

Open
JackieTien97 wants to merge 2 commits into
masterfrom
rest-row-limit
Open

Enforce REST query row limit as hard cap#18149
JackieTien97 wants to merge 2 commits into
masterfrom
rest-row-limit

Conversation

@JackieTien97

Copy link
Copy Markdown
Contributor

Summary

This PR makes rest_query_default_row_size_limit the hard upper bound for REST and Grafana query responses.

Changes

  • Cap user-provided REST rowLimit / row_limit with rest_query_default_row_size_limit.
  • Apply the same limit to Grafana variables, node, and expression endpoints instead of passing an unlimited 0.
  • Enforce the cap in v2 fast-last cache-hit responses, which previously built the response directly.
  • Centralize row-limit calculation and exceeded-limit response handling in QueryRowLimitUtils.
  • Update OpenAPI/config comments to describe the setting as a maximum response row limit.
  • Add focused regression tests for hard-cap resolution and exact-limit dataset handling.

Validation

  • mvn spotless:apply -pl external-service-impl/rest
  • mvn test -pl external-service-impl/rest -Dtest=QueryRowLimitUtilsTest,QueryDataSetHandlerTest
  • mvn test-compile -pl external-service-impl/rest -P with-zh-locale -DskipTests
  • mvn test-compile -pl external-service-impl/rest-openapi -DskipTests
  • git diff --check

@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 31.16883% with 106 lines in your changes missing coverage. Please review.
✅ Project coverage is 41.65%. Comparing base (e44cbbb) to head (295ef2f).
⚠️ Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
.../rest/protocol/v2/handler/QueryDataSetHandler.java 0.00% 38 Missing ⚠️
.../rest/protocol/v1/handler/QueryDataSetHandler.java 7.89% 35 Missing ⚠️
...b/rest/protocol/v1/impl/GrafanaApiServiceImpl.java 0.00% 8 Missing ⚠️
...b/rest/protocol/v2/impl/GrafanaApiServiceImpl.java 0.00% 8 Missing ⚠️
...protocol/table/v1/handler/QueryDataSetHandler.java 0.00% 6 Missing ⚠️
...est/protocol/table/v1/impl/RestApiServiceImpl.java 0.00% 4 Missing ⚠️
...otdb/rest/protocol/v2/impl/RestApiServiceImpl.java 0.00% 4 Missing ⚠️
...otdb/rest/protocol/v1/impl/RestApiServiceImpl.java 0.00% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #18149      +/-   ##
============================================
- Coverage     41.87%   41.65%   -0.22%     
- Complexity      318      343      +25     
============================================
  Files          5297     5328      +31     
  Lines        374093   376418    +2325     
  Branches      48287    48690     +403     
============================================
+ Hits         156645   156809     +164     
- Misses       217448   219609    +2161     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…n tests

A non-positive rest_query_default_row_size_limit used to mean "unlimited";
the hard-cap handling clamped it down to 1, silently capping every REST
response to a single row. Remap non-positive values to the built-in
default (10000) so existing deployments are not degraded, while still
guaranteeing a finite cap.

Add regression coverage:
- QueryRowLimitUtilsTest: an oversized caller-provided rowLimit is clamped
  to the configured hard limit.
- FastLastHandlerTest: the fastLastQuery cache-hit path caps the number of
  materialized entries. The cache-hit loop is extracted into
  FastLastHandler.fillLastValueDataSet so it is unit-testable.
@JackieTien97 JackieTien97 marked this pull request as ready for review July 8, 2026 08:11
@sonarqubecloud

sonarqubecloud Bot commented Jul 8, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
36.5% Duplication on New Code (required ≤ 5%)

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant