Skip to content
This repository has been archived by the owner on Sep 9, 2024. It is now read-only.

mock: validate empty partition keys for all operations #87

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

slai
Copy link

@slai slai commented Mar 28, 2023

The mock currently only validates empty partition keys for the Set operation (code), leading to unexpected success when empty partition keys are mistakenly used for Delete/Update/Read operations in tests. When testing against a real C* or Keyspaces instance, a Key may not be empty error will appear, code 0x2200.

This PR copies the logic used for the Set operation over to the other operations. I kept the same error message, even though it doesn't match the C* one. For clarity, note that -

  • a single partition key cannot be an empty string
  • a composite partition key can have one or all empty string values
  • a single or composite clustering columns can have one or all empty string values

I've added tests for these for the Map and MultiMap table types, but couldn't for the MultiMapMultiKey table type because there's a bug with the implementation there that eliminates empty key values even when they're allowed by C* and Keyspaces. Changing that would change app behaviour, so I've left that as-is, with a few added comments.

@slai slai force-pushed the slai/mock_validate_empty_partition_keys branch from 65cdcec to 9a0641b Compare March 28, 2023 12:33
The mock currently only validates empty partition keys for the Set operation, leading to unexpected success when empty partition keys are mistakenly used for Delete/Update/Read operations in tests. When testing against a real C* or Keyspaces instance, a `Key may not be empty` error will appear, code `0x2200`.

This PR copies the logic used for the Set operation over to the other operations. I kept the same error message, even though it doesn't match the C* one. For clarity, note that -

* a single partition key cannot be an empty string
* a composite partition key can have one or all empty string values
* a single or composite clustering columns can have one or all empty string values

I've added tests for these for the Map and MultiMap table types, but couldn't for the MultiMapMultiKey table type because there's a bug with the implementation there that eliminates empty key values even when they're allowed by C* and Keyspaces. Changing that would change app behaviour, so I've left that as-is, with a few added comments.
@slai slai force-pushed the slai/mock_validate_empty_partition_keys branch from a583ab7 to 8ebcaef Compare March 28, 2023 14:33
@slai slai marked this pull request as ready for review March 28, 2023 14:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant