feat: add support for clipped client ID lists and enhance aggregation handling

[ttypic]

Introduce clipped field in SummaryClientIdList and SummaryClientIdCounts for managing truncated client ID lists. Update relevant methods to read this field and handle additional aggregation metadata (totalClientIds, totalUnidentified). Add comprehensive test cases for new functionality.

Summary by CodeRabbit

• New Features

  • Summary responses now include clipped status for clientId lists and additional totals: totalClientIds and totalUnidentified.
  • Improved parsing to safely handle optional/missing summary fields.

• Documentation

  • Expanded model docs describing the new fields and aggregation semantics.

• Tests

  • Added tests covering clipped behavior and validation of totalClientIds and totalUnidentified across single and multiple-entry scenarios.

[coderabbitai]

Caution

Review failed

The pull request is closed.

Walkthrough

Parsers and models for V1 summary data now use named JSON keys, safely handle optional boolean/int fields, and surface clipping metadata. Two model classes gained new fields and constructors. Tests were added/extended to validate clipped, totalUnidentified, and totalClientIds behaviors.

Changes

Cohort / File(s)	Summary of Changes
Summary parsing updates lib/src/main/java/io/ably/lib/types/Summary.java	Added private JSON key constants (TOTAL, CLIENT_IDS, CLIPPED, TOTAL_UNIDENTIFIED, TOTAL_CLIENT_IDS); introduced tryReadBooleanField and tryReadIntField helpers; updated asSummaryMultipleV1, asSummaryFlagV1, and asSummaryTotalV1 to read new keys and populate extended model fields while handling missing values safely.
Model: counts/list lib/src/main/java/io/ably/lib/types/SummaryClientIdCounts.java, lib/src/main/java/io/ably/lib/types/SummaryClientIdList.java	SummaryClientIdCounts: added public final int totalUnidentified, public final boolean clipped, public final int totalClientIds; constructor expanded to accept these fields and Javadoc updated. SummaryClientIdList: added public final boolean clipped; constructor expanded and Javadoc updated.
Tests lib/src/test/java/io/ably/lib/types/SummaryTest.java	Added tests covering clipped=true for flag and multiple-entry V1 summaries; extended assertions to check totalUnidentified, totalClientIds, and clipped propagation; added assertFalse import.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Caller
  participant Summary
  participant JSON as JsonObject

  Caller->>Summary: asSummaryMultipleV1(JSON)
  Summary->>JSON: read TOTAL, CLIENT_IDS, CLIPPED, TOTAL_UNIDENTIFIED, TOTAL_CLIENT_IDS
  Note right of Summary #ddeeff: Missing numeric fields → null/0 as handled by helpers
  Summary-->>Caller: SummaryClientIdCounts(total, clientIds, totalUnidentified, totalClientIds, clipped)

  Caller->>Summary: asSummaryFlagV1(JSON)
  Summary->>JSON: read CLIENT_IDS, CLIPPED, TOTAL
  Summary-->>Caller: SummaryClientIdList(total, clientIds, clipped)

  Caller->>Summary: asSummaryTotalV1(JSON)
  Summary->>JSON: read TOTAL
  Summary-->>Caller: int total

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

A rabbit sniffs at JSON keys,
Totals counted in springy leaps.
Clipped or full, the lists hop through,
Unidentified join the crew.
Hooray — summaries snug and new! 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 7.14% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Linked Issues Check	❓ Inconclusive	The changes implement summary-related Protocol v4 behavior by adding the clipped flag and additional aggregation metadata and corresponding tests, which are relevant to ECO-5564's goal of supporting v4 aggregation semantics; however the linked issue explicitly calls out handling Protocol v4's new nested Message/ProtocolMessage structure and the provided summaries do not show modifications to Message or ProtocolMessage parsing, so I cannot confirm full compliance with all linked-issue objectives from the information given.	Please confirm whether nested Message/ProtocolMessage parsing changes are included in this PR or provide the file diffs/tests that cover those nested-structure changes, or state that those parts are handled in a separate PR so we can mark full compliance.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Out of Scope Changes Check	✅ Passed	All modified files in the raw summary are summary-related classes and tests (Summary.java, SummaryClientIdCounts, SummaryClientIdList, and SummaryTest) and the changes (new fields, parsing helpers, and tests) align with the PR objective to enhance aggregation handling and clipped lists, with no unrelated modules or public API surface changes indicated.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title succinctly describes the introduction of clipped client ID list support and enhancements to aggregation handling, directly reflecting the main objectives of the changeset. It clearly communicates the primary feature added without unnecessary detail or noise, making it easily understandable in project history. The phrasing aligns with conventional commit style and focuses on the most significant changes.

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 132470a and d0f1060.

📒 Files selected for processing (4)

• lib/src/main/java/io/ably/lib/types/Summary.java (2 hunks)
• lib/src/main/java/io/ably/lib/types/SummaryClientIdCounts.java (1 hunks)
• lib/src/main/java/io/ably/lib/types/SummaryClientIdList.java (1 hunks)
• lib/src/test/java/io/ably/lib/types/SummaryTest.java (3 hunks)

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 5

🧹 Nitpick comments (1)

lib/src/main/java/io/ably/lib/types/Summary.java (1)

86-94: Use typed deserialisation for List and validate clientIds shape.

Raw List.class can mask type issues. Also, add a clear error when clientIds is missing or not an array.

Apply:

-        List<String> clientIds = Serialisation.gson.fromJson(jsonObject.get(CLIENT_IDS), List.class);
+        if (jsonObject.get(CLIENT_IDS) == null || !jsonObject.get(CLIENT_IDS).isJsonArray()) {
+            throw new IllegalStateException("clientIds must be a JSON array");
+        }
+        List<String> clientIds = Serialisation.gson.fromJson(
+            jsonObject.get(CLIENT_IDS),
+            new com.google.gson.reflect.TypeToken<java.util.List<String>>() {}.getType()
+        );

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 8acadeb and 132470a.

📒 Files selected for processing (4)

• lib/src/main/java/io/ably/lib/types/Summary.java (2 hunks)
• lib/src/main/java/io/ably/lib/types/SummaryClientIdCounts.java (1 hunks)
• lib/src/main/java/io/ably/lib/types/SummaryClientIdList.java (1 hunks)
• lib/src/test/java/io/ably/lib/types/SummaryTest.java (3 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

lib/src/main/java/io/ably/lib/types/Summary.java (1)

lib/src/main/java/io/ably/lib/util/Serialisation.java (1)

• Serialisation (40-299)

lib/src/test/java/io/ably/lib/types/SummaryTest.java (1)

lib/src/main/java/io/ably/lib/types/Summary.java (1)

• Summary (30-176)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)

• GitHub Check: check
• GitHub Check: check-rest-okhttp
• GitHub Check: check-realtime
• GitHub Check: check-rest
• GitHub Check: check-realtime-okhttp
• GitHub Check: check-liveobjects
• GitHub Check: check (24)
• GitHub Check: check (19)
• GitHub Check: check (21)
• GitHub Check: check (29)
• GitHub Check: build
• GitHub Check: check (29)

🔇 Additional comments (6)

lib/src/main/java/io/ably/lib/types/SummaryClientIdList.java (1)

5-23: Docs: clear and useful context.

The added Javadoc clarifies semantics and the new clipped flag well.

lib/src/main/java/io/ably/lib/types/Summary.java (3)

34-39: Good: centralised JSON keys.

Moving literals to constants improves maintainability and reduces typos.

---

101-116: Helpers: sensible defaults.

The boolean/int field readers are straightforward and safe.

---

68-82: Use clientIds.size() as the fallback for totalClientIds, not total

-                totalClientIds == null ? total : totalClientIds
+                totalClientIds == null ? clientIds.size() : totalClientIds

Update any tests expecting the old value (e.g., 5/2 → 2/2).

lib/src/test/java/io/ably/lib/types/SummaryTest.java (2)

130-131: Nice: verifies clipped defaults to false.

---

234-253: Good coverage for clipped=true in multiple.v1.

Covers total, totalClientIds, totalUnidentified, and clipped propagation.

[AndyTWF]

Spec PR now at ably/specification#382 if we want to annotate :)