feat: rust backend api7 - #555
Merged
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (12)
rust/crates/adc-backend-api7/src/typing.rs (1)
268-271: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConsider tolerating a missing
listfield.
ListResponse::listhas no#[serde(default)]. If an API7 collection endpoint omitslistfor an empty collection, everyFetcher::listcall fails with a deserialization error instead of returning an empty vector. A default keeps the dump path working in that case.♻️ Proposed defensive change
#[derive(Debug, Clone, Deserialize)] pub struct ListResponse<T> { + #[serde(default)] pub list: Vec<T>, }Note:
Vec<T>: Defaultholds for anyT, so no extra bound is needed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/adc-backend-api7/src/typing.rs` around lines 268 - 271, Update the list field in ListResponse<T> to use Serde’s default for missing fields, so omitted collection responses deserialize with an empty Vec while preserving existing behavior when list is present.rust/crates/adc-backend-api7/src/operator.rs (1)
325-328: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winStamp the upstream id, as every other branch does.
Each other create/update branch sets the resource id from
event.resource_idbefore conversion. TheUpstreambranch does not, so the body'sidstays whatever the event payload carried, includingNone. Set it for consistency and to keep the body and the path in agreement.♻️ Proposed change
ResourceType::Upstream => { - let upstream: adc::Upstream = deserialize_event_value(new_value)?; + let mut upstream: adc::Upstream = deserialize_event_value(new_value)?; + upstream.id = Some(event.resource_id.clone()); to_request_body(typing::Upstream::from(upstream)) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/adc-backend-api7/src/operator.rs` around lines 325 - 328, Update the ResourceType::Upstream branch to assign event.resource_id to the deserialized upstream before converting it with typing::Upstream::from, matching the id-stamping behavior of the other create/update branches and keeping the request body id aligned with the path.rust/crates/adc-backend-api7/src/transformer.rs (1)
420-445: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueAn SSL with no certificates serializes as an empty
cert/keypair.If
ssl.certificatesis empty,firstbecomes an empty certificate and the wire body sendscert: ""andkey: "". The dashboard then answers with a schema error that does not name the real cause. Consider omittingcert/keywhen no certificate exists, so the error stays closer to "certificate missing".♻️ Proposed change
- let mut certificates = ssl.certificates.into_iter(); - let first = certificates.next().unwrap_or(adc::SSLCertificate { - certificate: String::new(), - key: String::new(), - }); + let mut certificates = ssl.certificates.into_iter(); + let first = certificates.next(); @@ - cert: Some(first.certificate), + cert: first.as_ref().map(|c| c.certificate.clone()), @@ - key: Some(first.key), + key: first.map(|c| c.key),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/adc-backend-api7/src/transformer.rs` around lines 420 - 445, Update the From<adc::SSL> for typing::Ssl implementation to detect when ssl.certificates is empty and omit the wire cert and key fields instead of serializing empty strings. Preserve the existing first-certificate mapping and certs/keys behavior when at least one certificate exists.rust/crates/adc-backend-api7/src/validator.rs (1)
96-121: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
unwrap_errhere depends onrequire_successrejecting every remaining status.The
_arm reachesunwrap_errfor all statuses outside 200-299 and 400. Ifrequire_successever accepts a status in that set, this line panics instead of returning an error. Match on the result instead, and return a descriptive error for the unexpectedOkcase.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/adc-backend-api7/src/validator.rs` around lines 96 - 121, Update the fallback status arm in the validator response match to handle HttpClient::require_success without calling unwrap_err. Match on its Result, return the existing error for Err, and construct a descriptive BackendError for an unexpected Ok response so no status can cause a panic.rust/crates/adc-backend-api7/src/default_value.rs (1)
47-63: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueThe upstream patch misses a service schema declared through
allOf.
patch_missing_upstream_schemaruns before theallOfmerge infetchand readsservice.properties.upstreamdirectly. If a release declares the service schema as anallOfcomposition,propertiesis absent at the top level, the patch returns early, and upstream defaults stay empty on exactly the older versions this function targets. Consider mergingallOffirst, or falling back to the merged service schema here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/adc-backend-api7/src/default_value.rs` around lines 47 - 63, Update patch_missing_upstream_schema to resolve the service schema after its allOf composition is merged, or otherwise fall back to that merged representation before reading service.properties.upstream. Preserve the existing early return when upstream already exists and continue inserting the patched object schema into the top-level schema.rust/crates/adc-backend-api7/src/fetcher.rs (1)
71-75: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBound the cascading fan-out concurrency.
Both calls pass
Noneas the concurrency limit, so every service and every consumer issues its follow-up requests at once. A gateway group with thousands of services opens thousands of simultaneous connections against the dashboard during onedump. Pass a fixed cap, or thread the sync concurrency option through to the fetcher.Also applies to: 126-132
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/adc-backend-api7/src/fetcher.rs` around lines 71 - 75, Update the concurrent_map_until_err calls in the fetcher flows, including the service path around with_upstreams_and_routes and the corresponding consumer path, to use a bounded concurrency limit instead of None. Prefer threading the existing sync concurrency option through the fetcher; otherwise use a fixed cap consistently for both cascading request fan-outs.rust/crates/adc-backend-api7/tests/common/mod.rs (1)
120-152: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInclude the response body in the bootstrap failure messages.
putandpostassert on the status only. A dashboard bootstrap failure then reports just the status code, which makes a broken login, license, or token step hard to diagnose. Read the body text before the assertion and add it to the panic message.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/adc-backend-api7/tests/common/mod.rs` around lines 120 - 152, Update the bootstrap helper methods put and post to read the response body text before asserting success, and include that text in the assertion failure message alongside the status. Preserve the existing request and JSON-decoding behavior, ensuring body-read failures identify the corresponding PUT or POST path.rust/crates/adc-backend-api7/tests/e2e_sync_and_dump_2.rs (1)
27-28: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssert the collection lengths before indexing.
The test indexes
ssls[0],services[0], andservices[1]without a length check. If the dump returns fewer entries, the failure is an index panic that does not name the missing resource. Addassert_eq!on the lengths first, as the other test files do.🔧 Proposed fix
let ssls = dump.ssls.as_ref().unwrap(); + assert_eq!(ssls.len(), 1); let mut ssl0 = serde_json::to_value(&ssls[0]).unwrap(); @@ let mut services = dump.services.take().unwrap(); + assert_eq!(services.len(), 2); services.sort_by(|a, b| a.name.cmp(&b.name));Also applies to: 44-45
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/adc-backend-api7/tests/e2e_sync_and_dump_2.rs` around lines 27 - 28, In the e2e dump test, assert the expected lengths of the SSL and service collections before indexing them: verify ssls has the required first entry and services has at least two entries before accessing ssls[0], services[0], or services[1]. Use assert_eq! with the expected counts and retain the existing indexing logic afterward.rust/crates/adc-backend-api7/tests/e2e_sync_and_dump_1.rs (1)
208-211: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSorting by
idmakes the expected order opaque.The assertions expect
route2beforeroute1only because the generated ids happen to order that way. A reader cannot verify this without computing the ids. Sort by a field the test controls, for exampleserver_port, and keep the expectations in that order.♻️ Proposed change
- stream_routes.sort_by(|a, b| a.id.cmp(&b.id)); + stream_routes.sort_by_key(|r| r.server_port); assert_eq!(stream_routes.len(), 2); - assert_matches_object(&serde_json::to_value(&stream_routes[0]).unwrap(), &route2); - assert_matches_object(&serde_json::to_value(&stream_routes[1]).unwrap(), &route1); + assert_matches_object(&serde_json::to_value(&stream_routes[0]).unwrap(), &route2); // 3306 + assert_matches_object(&serde_json::to_value(&stream_routes[1]).unwrap(), &route1); // 5432🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/adc-backend-api7/tests/e2e_sync_and_dump_1.rs` around lines 208 - 211, Update the stream_routes sorting in the test to use the controlled server_port field instead of id, then preserve the expected route2 and route1 assertions in the resulting server_port order..github/workflows/e2e.yaml (1)
192-223: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffThe image/license setup duplicates the
api7job.Lines 203-233 repeat lines 149-179 exactly, including the version matrix. A version or image change must then be applied in two places. Extract the shared steps into a composite action under
.github/actions/, or define the matrix once and reuse it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/e2e.yaml around lines 192 - 223, Deduplicate the API7 image and license setup shared by the api7 and api7-rust jobs: extract the repeated “Determine API7 image and license” steps into a composite action under .github/actions/ and invoke it from both jobs, while preserving the existing version matrix and dev-versus-release environment values.rust/crates/adc-backend-api7/tests/e2e_resource_route.rs (1)
20-63: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBoth tests repeat the same sync/dump/delete sequence.
The two tests differ only in the route payload and the version gate. Extract a helper that takes the route JSON, performs the sync, asserts the dump, and deletes the service. The cleanup also runs only on the success path. If an assertion panics, the
testservice stays in the dashboard and can affect later tests in the serial run. A shared helper with a guard that deletes the service on drop removes both problems.Also applies to: 67-114
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/adc-backend-api7/tests/e2e_resource_route.rs` around lines 20 - 63, Extract the duplicated sync/dump/delete workflow from both route timeout tests into a shared helper that accepts the route JSON and handles the applicable version gate. Have the helper assert the dumped service and route data, while using a drop guard to delete the test service even when an assertion panics; update both tests to call this helper with their respective payloads.rust/crates/adc-backend-api7/tests/e2e_default_value.rs (1)
27-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueVersion gates report success when they skip.
Each gated test prints to stderr and returns. A skipped test is reported as passing, so a misconfigured
BACKEND_API7_VERSIONhides all three tests silently. Consider asserting that at least one branch matched, for example by centralizing the gate incommonand counting executed branches, or by failing whenserver_version()equals the fallback0.0.0.Also applies to: 76-79, 96-99
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/adc-backend-api7/tests/e2e_default_value.rs` around lines 27 - 30, Update the version gates in the three tests in e2e_default_value.rs so an unrecognized or fallback server version cannot silently return as a passing test. Centralize the gate through the existing common test utilities or otherwise track whether a supported version branch executed, and fail when server_version() is the fallback 0.0.0; preserve the intended skip behavior for explicitly unsupported versions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/e2e.yaml:
- Line 225: Update the actions/checkout step in the workflow to disable
credential persistence while preserving the existing pinned action version and
read-only clone behavior; configure checkout with persist-credentials set to
false.
In `@rust/crates/adc-backend-api7/src/fetcher.rs`:
- Around line 188-239: The Fetcher::dump method currently attaches nested
service routes regardless of config::filter_resource_types. Apply the configured
resource-type filter before assigning service.routes, excluding HTTP routes and
stream routes when Route or StreamRoute is filtered out, and ensure other
excluded nested resources such as Upstream or ConsumerCredential are not
returned through services. Preserve service output when Service remains enabled.
In `@rust/crates/adc-backend-api7/src/gateway_group.rs`:
- Around line 55-66: Update GatewayGroupSummary to decode the gateway group
name, then change the lookup in the gateway-group resolution flow to select the
entry whose name exactly matches self.name rather than taking the first result.
Preserve the existing missing-group BackendError when no exact match is found,
and continue returning the matched group’s id.
In `@rust/crates/adc-backend-api7/src/transformer.rs`:
- Around line 363-377: Update transform_service to derive ty as "stream" when
either the upstream scheme is Tcp, Udp, or Tls, or the service routes are
ServiceRoutes::Stream; otherwise retain "http". Ensure stream routes without an
upstream are serialized with the stream type so downstream fetch logic preserves
them.
In `@rust/crates/adc-backend-api7/tests/common/mod.rs`:
- Around line 202-214: Update bootstrap_token to obtain the backend version by
reusing server_version() instead of independently parsing BACKEND_API7_VERSION
with a 0.0.0 fallback. Remove the duplicate environment parsing so unset and
invalid values follow the same default and validation behavior as
server_version(), while preserving the existing init_user call and license
handling.
In `@rust/crates/adc-backend-api7/tests/e2e_default_value.rs`:
- Around line 15-22: Update the default-value setup used by fetch and
core_default so schema defaults remain available when transform_default cannot
deserialize partial values such as Consumer’s empty object. Preserve these
entries as schema-only partial values without requiring identity fields, or
process them before typed conversion, and ensure core_default returns the
preserved default instead of panicking on an omitted resource.
In `@rust/crates/adc-backend-api7/tests/e2e_ping.rs`:
- Around line 24-44: Update the HttpClientConfig server value in
ping_fails_against_an_unreachable_server to use http://127.0.0.1:1 instead of
http://0.0.0.0, preserving the existing connection-error assertion and test
setup.
In `@rust/crates/adc-backend-api7/tests/e2e_resource_consumer.rs`:
- Around line 53-76: Update the consumer assertions in the test around
dump_configuration to select entries by their username rather than assuming list
indices. Preserve validation of both consumer1 and consumer2 before the update,
and locate the updated consumer1 by username before matching its contents.
In `@rust/crates/adc-backend-apisix/src/fetcher.rs`:
- Around line 157-160: Update the consumer fetch flow around
concurrent_map_until_err and self.with_credentials to check
self.filter.is_skip(ResourceType::ConsumerCredential) first; when credentials
are excluded, return consumers unchanged, otherwise preserve the existing
credential-fetch behavior.
In `@rust/crates/adc-cli/src/pipeline.rs`:
- Around line 136-140: Update the pipeline around config::fill_labels and
config::inject_managed_by_label so inject_managed_by_label runs first, allowing
the explicit managed-by selector value to remain authoritative. Preserve the
existing remote selector behavior and add a regression test covering
--managed-by-label together with --label-selector managed-by=<value>, verifying
local labels match the selector.
---
Nitpick comments:
In @.github/workflows/e2e.yaml:
- Around line 192-223: Deduplicate the API7 image and license setup shared by
the api7 and api7-rust jobs: extract the repeated “Determine API7 image and
license” steps into a composite action under .github/actions/ and invoke it from
both jobs, while preserving the existing version matrix and dev-versus-release
environment values.
In `@rust/crates/adc-backend-api7/src/default_value.rs`:
- Around line 47-63: Update patch_missing_upstream_schema to resolve the service
schema after its allOf composition is merged, or otherwise fall back to that
merged representation before reading service.properties.upstream. Preserve the
existing early return when upstream already exists and continue inserting the
patched object schema into the top-level schema.
In `@rust/crates/adc-backend-api7/src/fetcher.rs`:
- Around line 71-75: Update the concurrent_map_until_err calls in the fetcher
flows, including the service path around with_upstreams_and_routes and the
corresponding consumer path, to use a bounded concurrency limit instead of None.
Prefer threading the existing sync concurrency option through the fetcher;
otherwise use a fixed cap consistently for both cascading request fan-outs.
In `@rust/crates/adc-backend-api7/src/operator.rs`:
- Around line 325-328: Update the ResourceType::Upstream branch to assign
event.resource_id to the deserialized upstream before converting it with
typing::Upstream::from, matching the id-stamping behavior of the other
create/update branches and keeping the request body id aligned with the path.
In `@rust/crates/adc-backend-api7/src/transformer.rs`:
- Around line 420-445: Update the From<adc::SSL> for typing::Ssl implementation
to detect when ssl.certificates is empty and omit the wire cert and key fields
instead of serializing empty strings. Preserve the existing first-certificate
mapping and certs/keys behavior when at least one certificate exists.
In `@rust/crates/adc-backend-api7/src/typing.rs`:
- Around line 268-271: Update the list field in ListResponse<T> to use Serde’s
default for missing fields, so omitted collection responses deserialize with an
empty Vec while preserving existing behavior when list is present.
In `@rust/crates/adc-backend-api7/src/validator.rs`:
- Around line 96-121: Update the fallback status arm in the validator response
match to handle HttpClient::require_success without calling unwrap_err. Match on
its Result, return the existing error for Err, and construct a descriptive
BackendError for an unexpected Ok response so no status can cause a panic.
In `@rust/crates/adc-backend-api7/tests/common/mod.rs`:
- Around line 120-152: Update the bootstrap helper methods put and post to read
the response body text before asserting success, and include that text in the
assertion failure message alongside the status. Preserve the existing request
and JSON-decoding behavior, ensuring body-read failures identify the
corresponding PUT or POST path.
In `@rust/crates/adc-backend-api7/tests/e2e_default_value.rs`:
- Around line 27-30: Update the version gates in the three tests in
e2e_default_value.rs so an unrecognized or fallback server version cannot
silently return as a passing test. Centralize the gate through the existing
common test utilities or otherwise track whether a supported version branch
executed, and fail when server_version() is the fallback 0.0.0; preserve the
intended skip behavior for explicitly unsupported versions.
In `@rust/crates/adc-backend-api7/tests/e2e_resource_route.rs`:
- Around line 20-63: Extract the duplicated sync/dump/delete workflow from both
route timeout tests into a shared helper that accepts the route JSON and handles
the applicable version gate. Have the helper assert the dumped service and route
data, while using a drop guard to delete the test service even when an assertion
panics; update both tests to call this helper with their respective payloads.
In `@rust/crates/adc-backend-api7/tests/e2e_sync_and_dump_1.rs`:
- Around line 208-211: Update the stream_routes sorting in the test to use the
controlled server_port field instead of id, then preserve the expected route2
and route1 assertions in the resulting server_port order.
In `@rust/crates/adc-backend-api7/tests/e2e_sync_and_dump_2.rs`:
- Around line 27-28: In the e2e dump test, assert the expected lengths of the
SSL and service collections before indexing them: verify ssls has the required
first entry and services has at least two entries before accessing ssls[0],
services[0], or services[1]. Use assert_eq! with the expected counts and retain
the existing indexing logic afterward.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5d59f7b2-9dbb-438c-b2c9-c537d2a079f1
⛔ Files ignored due to path filters (1)
rust/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (44)
.github/workflows/e2e.yamlrust/Cargo.tomlrust/crates/adc-backend-api7/Cargo.tomlrust/crates/adc-backend-api7/src/backend.rsrust/crates/adc-backend-api7/src/default_value.rsrust/crates/adc-backend-api7/src/fetcher.rsrust/crates/adc-backend-api7/src/gateway_group.rsrust/crates/adc-backend-api7/src/lib.rsrust/crates/adc-backend-api7/src/operator.rsrust/crates/adc-backend-api7/src/transformer.rsrust/crates/adc-backend-api7/src/typing.rsrust/crates/adc-backend-api7/src/utils.rsrust/crates/adc-backend-api7/src/validator.rsrust/crates/adc-backend-api7/tests/common/mod.rsrust/crates/adc-backend-api7/tests/e2e_default_value.rsrust/crates/adc-backend-api7/tests/e2e_gateway_group.rsrust/crates/adc-backend-api7/tests/e2e_misc.rsrust/crates/adc-backend-api7/tests/e2e_ping.rsrust/crates/adc-backend-api7/tests/e2e_resource_consumer.rsrust/crates/adc-backend-api7/tests/e2e_resource_route.rsrust/crates/adc-backend-api7/tests/e2e_resource_service_upstream.rsrust/crates/adc-backend-api7/tests/e2e_stream_route_plugins.rsrust/crates/adc-backend-api7/tests/e2e_sync_and_dump_1.rsrust/crates/adc-backend-api7/tests/e2e_sync_and_dump_2.rsrust/crates/adc-backend-api7/tests/e2e_validate.rsrust/crates/adc-backend-api7/tests/timeout.rsrust/crates/adc-backend-api7/tests/validator.rsrust/crates/adc-backend-apisix/src/backend.rsrust/crates/adc-backend-apisix/src/fetcher.rsrust/crates/adc-backend-apisix/src/utils.rsrust/crates/adc-backend-apisix/tests/common/mod.rsrust/crates/adc-backend-apisix/tests/e2e_apisix.rsrust/crates/adc-backend-apisix/tests/e2e_ping.rsrust/crates/adc-backend-apisix/tests/e2e_resource_consumer.rsrust/crates/adc-backend-apisix/tests/e2e_resource_service.rsrust/crates/adc-backend-apisix/tests/e2e_validate.rsrust/crates/adc-backend-core/src/client.rsrust/crates/adc-backend-core/src/lib.rsrust/crates/adc-backend-core/src/resource_filter.rsrust/crates/adc-backend-core/src/resource_path.rsrust/crates/adc-cli/Cargo.tomlrust/crates/adc-cli/src/config.rsrust/crates/adc-cli/src/main.rsrust/crates/adc-cli/src/pipeline.rs
bzp2010
force-pushed
the
bzp/feat-rust-backend-api7
branch
from
August 5, 2026 14:27
dc5fe56 to
e2f0580
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
rust/crates/adc-backend-apisix/tests/transformer.rs (1)
139-139: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the converted
priorityvalue.The test changes
priority, but the assertions only checkhostandport. Addassert_eq!(nodes[0].priority, 0);to verify that the integer value survives conversion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/adc-backend-apisix/tests/transformer.rs` at line 139, Update the relevant transformer test assertions to validate the converted priority field as well as host and port. In the test using typing::UpstreamNodes::List and adc::UpstreamNode, assert that nodes[0].priority equals 0.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rust/crates/adc-backend-api7/src/default_value.rs`:
- Around line 222-245: The LenientSslClient conversion currently defaults an
omitted depth to 0 instead of the SDK default 1. Update LenientSslClient’s depth
deserialization to use a default function returning 1, and add coverage for
deserializing a client without depth to verify the converted adc::SslClient
preserves depth 1.
In `@rust/crates/adc-sdk/src/resources/common.rs`:
- Around line 41-44: Update the integer-conversion predicate in the visible
serializer branch to keep the lower bound inclusive while requiring the finite
value to be strictly less than 2^63, rather than comparing against i64::MAX as
f64. Add a regression test covering 2^63 to verify it is serialized as an f64
instead of being converted to i64.
In `@rust/crates/adc-sdk/src/resources/upstream.rs`:
- Around line 94-95: Update the resource deserialization around priority and
concurrency, including default_concurrency, to accept JSON integers and integral
decimal numbers such as 0.0 and 10.0 while preserving integer behavior; add and
apply suitable serde deserializers, then add regression coverage in
resources_from_fixtures.rs for both fields and numeric forms.
---
Nitpick comments:
In `@rust/crates/adc-backend-apisix/tests/transformer.rs`:
- Line 139: Update the relevant transformer test assertions to validate the
converted priority field as well as host and port. In the test using
typing::UpstreamNodes::List and adc::UpstreamNode, assert that nodes[0].priority
equals 0.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 310e476e-bd5d-4bce-af0b-e4c835c66548
📒 Files selected for processing (22)
.github/workflows/e2e.yamlrust/crates/adc-backend-api7/src/default_value.rsrust/crates/adc-backend-api7/src/gateway_group.rsrust/crates/adc-backend-api7/src/operator.rsrust/crates/adc-backend-api7/src/typing.rsrust/crates/adc-backend-api7/tests/common/mod.rsrust/crates/adc-backend-api7/tests/e2e_init.rsrust/crates/adc-backend-api7/tests/e2e_misc.rsrust/crates/adc-backend-api7/tests/e2e_sync_and_dump_1.rsrust/crates/adc-backend-api7/tests/e2e_sync_and_dump_2.rsrust/crates/adc-backend-api7/tests/e2e_validate.rsrust/crates/adc-backend-api7/tests/timeout.rsrust/crates/adc-backend-api7/tests/validator.rsrust/crates/adc-backend-apisix/src/transformer.rsrust/crates/adc-backend-apisix/src/typing.rsrust/crates/adc-backend-apisix/tests/transformer.rsrust/crates/adc-cli/src/config.rsrust/crates/adc-cli/src/pipeline.rsrust/crates/adc-sdk/src/resources/common.rsrust/crates/adc-sdk/src/resources/mod.rsrust/crates/adc-sdk/src/resources/upstream.rsrust/crates/adc-sdk/tests/resources_from_fixtures.rs
🚧 Files skipped from review as they are similar to previous changes (9)
- rust/crates/adc-backend-api7/tests/e2e_sync_and_dump_2.rs
- rust/crates/adc-backend-api7/src/typing.rs
- rust/crates/adc-backend-api7/tests/e2e_validate.rs
- rust/crates/adc-backend-api7/tests/e2e_sync_and_dump_1.rs
- rust/crates/adc-cli/src/pipeline.rs
- rust/crates/adc-cli/src/config.rs
- rust/crates/adc-backend-api7/tests/timeout.rs
- rust/crates/adc-backend-api7/tests/validator.rs
- rust/crates/adc-backend-api7/tests/common/mod.rs
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Backend for API7.
Checklist
Summary by CodeRabbit