[Bugfix] Preserve empty values in Mooncake Store REST GET

[VectorPeak]

Description

This PR fixes a value-preservation bug in the Mooncake Store REST API GET /api/get/{key} path. The handler previously treated an empty byte payload as a missing key, so a value that could be stored through the REST API could not be read back reliably.

The original REST read path used the value returned by self.store.get(key) to decide whether the key existed:

value = self.store.get(key)
if not value:
    return 404

That check conflates two different states: an existing empty byte payload (b"") and an absent object. Reviewer feedback also pointed out that the real MooncakeDistributedStore.get() binding may return zero-length bytes when the underlying get_buffer(key) result is null, so get() alone is not a reliable existence signal.

The updated handler now uses the store's explicit existence API before reading the value:

exists = self.store.is_exist(key)
if exists == 0:
    return 404
if exists < 0:
    return 500
value = self.store.get(key)

For empty byte values, the handler re-checks existence after get() returns b"". This keeps valid stored empty payloads readable while avoiding a false 200 response when the binding surfaces a missing/null buffer as zero-length bytes.

Changes Made

• Switched REST GET missing-key detection from value truthiness/None checks to self.store.is_exist(key).
• Preserved b"" as a valid 200 response body for existing keys.
• Returned 500 when is_exist() reports a negative error or when get() unexpectedly returns None after a successful existence check.
• Updated tests so missing keys match the real binding behavior: is_exist(key) == 0 while get(key) may still be b"".
• Added coverage for empty byte payloads, existence-check failure, unexpected None from get(), and the empty-byte ambiguity after get().

Evidence

The original bug can be reproduced with an isolated handler-style case where the store returns an existing empty byte payload:

self.fake_store.is_exist = lambda key: True
self.fake_store.get = lambda key: b""
request.match_info = {"key": "empty_value_key"}
resp = await self.service.handle_get(request)

Before this fix, the handler evaluated b"" as false and returned the missing-key branch:

status: 404
body: {"error": "Key not found"}

After this fix, the key existence check distinguishes an existing empty payload from a missing key:

status: 200
body: b""

A real missing key is represented by is_exist(key) == 0 and still returns 404 Key not found even if get(key) would produce b"".

Possible Call Chain / Impact

The affected path is the Mooncake Store REST API read flow:

mooncake-wheel/mooncake/mooncake_store_service.py
  -> MooncakeStoreService.start_http_service(...)
  -> web.get('/api/get/{key}', self.handle_get)
  -> MooncakeStoreService.handle_get(...)
  -> self.store.is_exist(key)
  -> self.store.get(key)
  -> web.Response(body=value, content_type='application/octet-stream')

This PR only changes the REST GET response decision for values returned through the REST shim. It does not change writes, deletion, mounting, store setup, or the underlying Mooncake distributed store APIs.

Module

• Transfer Engine (mooncake-transfer-engine)
• Mooncake Store (mooncake-store)
• Mooncake EP (mooncake-ep)
• Mooncake PG (mooncake-pg)
• Integration (mooncake-integration)
• P2P Store (mooncake-p2p-store)
• Python Wheel (mooncake-wheel)
• Common (mooncake-common)
• Mooncake RL (mooncake-rl)
• CI/CD
• Docs
• Other

Type of Change

• Bug fix
• New feature
• Refactor
• Breaking change
• Documentation update
• Performance improvement
• Other

How Has This Been Tested?

Test commands:

python -m unittest mooncake-wheel.tests.test_mooncake_store_service_api.StoreServiceApiTest.test_handle_get_success mooncake-wheel.tests.test_mooncake_store_service_api.StoreServiceApiTest.test_handle_get_empty_bytes mooncake-wheel.tests.test_mooncake_store_service_api.StoreServiceApiTest.test_handle_get_empty_bytes_rechecks_existence mooncake-wheel.tests.test_mooncake_store_service_api.StoreServiceApiTest.test_handle_get_not_found mooncake-wheel.tests.test_mooncake_store_service_api.StoreServiceApiTest.test_handle_get_none_value_is_store_failure mooncake-wheel.tests.test_mooncake_store_service_api.StoreServiceApiTest.test_handle_get_exist_check_failure mooncake-wheel.tests.test_mooncake_store_service_api.StoreServiceApiTest.test_handle_get_store_exception
git diff --check

Test results:

• Unit tests pass
• Integration tests pass (if applicable)
• Manual testing done (describe below)

Targeted GET handler tests passed:

Ran 7 tests in 0.017s

OK

git diff --check passed with no patch errors. It emitted Windows line-ending warnings only.

I also ran the full mooncake-wheel.tests.test_mooncake_store_service_api file locally before this update. It had unrelated existing failures around handle_put_missing_value and unmount call expectations, so I did not include that as a passing validation claim.

Checklist

• I have performed a self-review of my own code
• I have formatted my code using ./scripts/code_format.sh
• I have run pre-commit run --all-files and all hooks pass
• I have updated the documentation (if applicable)
• I have added tests to prove my changes are effective
• For changes >500 LOC: I have filed an RFC issue (not applicable; this PR changes fewer than 100 lines)

AI Assistance Disclosure

• No AI tools were used
• AI tools were used (specify below)

AI assistance was used to identify the edge case, make the handler/test updates, respond to review feedback, and draft this PR description. The submitter is responsible for reviewing and validating the final changes.

[gemini-code-assist]

Code Review

This pull request updates the handle_get method in mooncake_store_service.py to check if the retrieved value is explicitly None instead of evaluating its truthiness. This allows empty values, such as empty bytes, to be successfully returned with a 200 OK status rather than incorrectly triggering a 404 Not Found error. The mock Response class and the test case test_handle_get_empty_bytes in the test suite have been updated to support and verify this new behavior. There are no review comments to address, and I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[Lin-z-w]

The fix assumes self.store.get(key) returns None for a missing key, but the real MooncakeDistributedStore.get() binding returns zero-length bytes when get_buffer(key) is null. With the new if value is None check, a real missing key falls through and returns 200 with an empty body instead of 404.

The updated unit test passes because the fake store models missing keys as None, which does not match the real binding behavior. Please either change the binding contract so missing keys return None, or make handle_get() distinguish existence separately, for example by checking is_exist(key) before returning b"" as a valid stored value.

[VectorPeak]

Thanks for catching this. I updated the REST GET path to use the binding's existence contract instead of using the value truthiness/None result to decide whether to return 404.

handle_get() now checks self.store.is_exist(key) first, returns 404 when it returns 0, returns 500 on a negative existence-check result, and only then calls get(). Empty bytes are preserved as a valid 200 response for existing keys. I also added a second existence check when get() returns b"" so the REST handler does not turn a missing/null buffer into a false 200 response.

The unit tests now model the real binding behavior: the missing-key case uses is_exist == 0 while get() may still be b"". I also added coverage for existence-check failure, unexpected None from get(), and the empty-byte ambiguity after get().

Validation:

Ran 7 focused GET handler tests in 0.017s

OK

git diff --check also passes locally, with only Windows LF/CRLF warnings.

[Lin-z-w]

Thanks for the fix. LGTM.