Don't expose underlying replay data buffer

[Sheikah45]

Summary by CodeRabbit

• New Features

  • Introduced a new public method providing direct access to raw replay data in byte array format

• Refactor

  • Enhanced encapsulation of internal replay buffer structures by restricting direct access and providing a dedicated method for data retrieval, improving API clarity and maintainability

[coderabbitai]

Walkthrough

A simple encapsulation refactor renames the public data ByteBuffer field to private dataBuffer, removing its generated getter. A new public method getRawReplayData() returns a byte array copy. Tests updated accordingly to use the new API method.

Changes

Cohort / File(s)	Summary
Core API Refactoring data/src/main/java/com/faforever/commons/replay/ReplayDataParser.java	Field data renamed to private dataBuffer, eliminating auto-generated getter. New public method getRawReplayData() returns byte array copy. All internal usages updated to reference dataBuffer.
Test Updates data/src/test/java/com/faforever/commons/replay/ReplayLoaderTableParserTest.java	Two test method calls replaced: getData().array() → getRawReplayData() to align with new public API.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

• Verify getRawReplayData() correctly creates a defensive copy of the buffer data
• Confirm all internal references to data have been updated to dataBuffer
• Ensure test assertions remain functionally equivalent after API change

Poem

🐰 A buffer once bare, now tucked safe away,
Behind private walls where the data shall stay,
A new method returns what you need, fair and clear,
Encapsulation's gift—no more leaks to fear! 🎀

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Don't expose underlying replay data buffer' accurately reflects the main change: encapsulating the private ByteBuffer field and replacing its public getter with a safer getRawReplayData() method that returns a copy.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch maintenance/encapsualte-replay-data

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 53532b5 and 3af9981.

📒 Files selected for processing (2)

• data/src/main/java/com/faforever/commons/replay/ReplayDataParser.java (3 hunks)
• data/src/test/java/com/faforever/commons/replay/ReplayLoaderTableParserTest.java (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

data/src/main/java/com/faforever/commons/replay/ReplayDataParser.java (1)

data/src/main/java/com/faforever/commons/replay/body/ReplayBodyParser.java (1)

• ReplayBodyParser (12-287)

🔇 Additional comments (3)

data/src/main/java/com/faforever/commons/replay/ReplayDataParser.java (2)

47-47: LGTM! Clean encapsulation refactor.

The field renaming from data to dataBuffer with private visibility and consistent internal reference updates improves encapsulation. All usages are correctly updated throughout the parsing logic.

Also applies to: 138-138, 444-452

---

456-459: Implementation verified—no issues found.

All buffer creation paths guarantee array-backed buffers through ByteBuffer.wrap(): initial buffer loading, ZSTD decompression, and QTCOMPRESS decompression all return array-backed buffers. The defensive copy via Arrays.copyOf() correctly prevents external modification of internal state. Tests pass and validate correctness through byte-by-byte comparison with reference files.

data/src/test/java/com/faforever/commons/replay/ReplayLoaderTableParserTest.java (1)

111-111: LGTM! Tests correctly updated to use the new API.

The test methods now use getRawReplayData() instead of getData().array(), which is cleaner and correctly uses the new encapsulated API. Test behavior and assertions remain unchanged.

Also applies to: 123-123

---

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