Skip to content

Object store get: no wait for an empty object, no NPE on a single chunk with no data - #1644

Merged
scottf merged 1 commit into
mainfrom
os-get-empty-object-and-null-data
Oct 21, 2026
Merged

scottf merged 1 commit into
mainfrom
os-get-empty-object-and-null-data

Conversation

@scottf

@scottf scottf commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Two read-side fixes in NatsObjectStore.get.

Empty object. put of an empty stream stores an object with 0 chunks. get only special-cased 1 chunk, so for 0 chunks it subscribed and waited in nextMessage(jsm.getTimeout()) for chunks that do not exist, then passed the size and digest checks. Every get of an empty object took the full JetStream timeout (2 s by default). The chunk read is now skipped when the object has no chunks.

Single chunk with no data. Without direct get, the message get API returns no data for an empty payload, so MessageInfo.getData() is null. The single-chunk branch passed it to digester.update(...), which threw a NullPointerException. The data is now digested and written only when present, so the existing size check reports the mismatch.

Tests, in ObjectStoreTests:

  • testEmptyObject: puts an empty object, asserts get returns before the JetStream timeout with size 0 and 0 chunks. Without the fix it fails, each get taking 2.1 to 2.3 s.
  • testSingleChunkWithNoData: puts a one-chunk object, turns off allowDirect on the stream, publishes an empty payload as the chunk, and asserts get throws the OsGetSizeMismatch error. Without the fix it fails with NullPointerException.

ObjectStoreTests: 13 tests, 0 failures.

🤖 Generated with Claude Code

…nk with no data

- An object with 0 chunks no longer subscribes and waits the JetStream timeout for chunks that do not exist.
- A single chunk read without direct get can have null data; it is no longer passed to the digester, so the size check reports the mismatch instead of a NullPointerException.
- New ObjectStoreTests.testEmptyObject and testSingleChunkWithNoData.
@synadia-claude-reviewer

synadia-claude-reviewer Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Claude finished @scottf's task in 58s —— View job


Review complete

  • Fetch PR diff and context
  • Review NatsObjectStore.get changes for correctness
  • Review new tests
  • Post review summary

Summary: Both fixes in NatsObjectStore.get (src/main/java/io/nats/client/impl/NatsObjectStore.java) are correct and well-scoped.

  • Empty object skip: Changing the fallback branch to else if (oi.getChunks() > 0) correctly avoids subscribing/waiting when there are no chunks. The post-loop checks (totalChunks != oi.getChunks(), totalBytes != oi.getSize(), digest match) still hold with totalChunks/totalBytes left at 0, since put always computes a digest (even over zero bytes) via digester.getDigestEntry() — so testEmptyObject's digest/size assertions are consistent with put's behavior.
  • Null-data NPE fix: MessageInfo.getData() is annotated @Nullable (confirmed in MessageInfo.java), so guarding digester.update(data)/out.write(data) with data != null is the right fix. Note the multi-chunk subscription loop (lines ~239-243) doesn't need the same guard — it reads from a regular Message, not MessageInfo, and a normal NATS protocol payload is never null (zero-length array at worst), so there's no parallel NPE risk there.
  • Size mismatch still correctly surfaces as OsGetSizeMismatch when a chunk's actual data length doesn't match the recorded object size, which is what testSingleChunkWithNoData verifies.

No security, thread-safety, or API-compatibility concerns. Tests look solid and exercise both fixed paths directly.

@aricart
aricart self-requested a review October 21, 2026 12:24
@scottf
scottf merged commit cae9561 into main Oct 21, 2026
8 checks passed
@scottf
scottf deleted the os-get-empty-object-and-null-data branch October 21, 2026 14:16
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.

2 participants