[ISSUE #10681] Skip lite offline cleanup without subscriptions

[Aias00]

Summary

• Skip lite-client offline cleanup when settings have no subscription section or no subscription entries.
• Avoid indexing subscriptions(0) for incomplete lite-consumer settings.
• Add a regression test for lite consumers without subscription entries.

Motivation

Closes #10681.

GrpcClientSettingsManager.offlineClientLiteSubscription previously assumed lite-consumer settings always contain at least one subscription entry. Incomplete settings could throw IndexOutOfBoundsException and fall into the broad error log path, including the full settings object.

Tests

• mvn -pl proxy -Dtest=GrpcClientSettingsManagerTest -DfailIfNoTests=false test

Result: BUILD SUCCESS; 8 tests passed; checkstyle reported 0 violations. The run emits existing JaCoCo instrumentation warnings under the local JDK, and one existing test intentionally logs a simulated sync failure, but tests and build completed successfully.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 48.23%. Comparing base (00e45b8) to head (46c9474).

Files with missing lines	Patch %	Lines
...roxy/grpc/v2/common/GrpcClientSettingsManager.java	50.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #10682      +/-   ##
=============================================
- Coverage      48.31%   48.23%   -0.09%     
+ Complexity     13511    13485      -26     
=============================================
  Files           1380     1380              
  Lines         101091   101093       +2     
  Branches       13101    13102       +1     
=============================================
- Hits           48844    48759      -85     
- Misses         46285    46346      +61     
- Partials        5962     5988      +26     

☔ 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.

[RockteMQ-AI]

Review by github-manager-bot

Summary

Adds an early-return guard in GrpcClientSettingsManager.offlineClientLiteSubscription() to skip processing when the lite-consumer settings have no subscription section or zero subscription entries, preventing an IndexOutOfBoundsException on getSubscriptions(0). Includes a regression test.

Findings

• [Info] GrpcClientSettingsManager.java:239 — The guard !settings.hasSubscription() || settings.getSubscription().getSubscriptionsCount() == 0 correctly prevents the IndexOutOfBoundsException described in #10681. Clean and minimal.
• [Info] GrpcClientSettingsManagerTest.java:159-172 — Good regression test using LITE_PUSH_CONSUMER with a group but no subscription entries. Verifies syncLiteSubscription is never called.

Suggestions

• Consider also testing with LITE_SIMPLE_CONSUMER client type for completeness, since the method guards both LITE_PUSH_CONSUMER and LITE_SIMPLE_CONSUMER at line 235-237. This is optional — the current test already validates the new guard logic.

Verdict

The fix is correct, minimal, and well-tested. LGTM. ✅

---

Automated review by github-manager-bot

[f1amingo]

Thanks for the PR. However, this guards a state that cannot occur by design.

In the liteTopic model, the bind topic is mandatory on the SDK side — a lite consumer always carries exactly one bind-topic entry in its Settings, established before any lite subscription exists. So getSubscriptions(0) is safe here by contract.

If this state ever appeared, it would signal a protocol violation that should fail loudly (which the existing catch + error log already does), not be silently skipped on the offline path.

I'd suggest walking through the end-to-end liteTopic subscription flow (Settings sync → bind topic → SyncLiteSubscription) first — it would make it clear why this invariant holds.

[Aias00]

Thanks for the PR. However, this guards a state that cannot occur by design.

In the liteTopic model, the bind topic is mandatory on the SDK side — a lite consumer always carries exactly one bind-topic entry in its Settings, established before any lite subscription exists. So getSubscriptions(0) is safe here by contract.

If this state ever appeared, it would signal a protocol violation that should fail loudly (which the existing catch + error log already does), not be silently skipped on the offline path.

I'd suggest walking through the end-to-end liteTopic subscription flow (Settings sync → bind topic → SyncLiteSubscription) first — it would make it clear why this invariant holds.

sounds good to me