Skip to content

Add compactStructuredEncryptionData administration command tests#625

Open
imforster wants to merge 1 commit into
documentdb:mainfrom
imforster:forstaia/administration/compactStructuredEncryptionData
Open

Add compactStructuredEncryptionData administration command tests#625
imforster wants to merge 1 commit into
documentdb:mainfrom
imforster:forstaia/administration/compactStructuredEncryptionData

Conversation

@imforster

@imforster imforster commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

#416
Test cases: 36
Docs: https://www.mongodb.com/docs/v8.2/reference/command/compactStructuredEncryptionData/

Summary

Adds comprehensive tests for compactStructuredEncryptionData administration command using proper DRY parametrization patterns.

Changes

  • error_codes.py: Add NOT_ENCRYPTED_COLLECTION_ERROR = 6346807 constant
  • field_validation.py: Uses bson_type_validator for exhaustive compactionTokens type rejection (16 BSON types), plus §19 namespace validation and missing field tests
  • core_behavior.py: Parametrized CommandTestCase tests for non-encrypted collection rejection
  • error_cases.py: Parametrized tests for unrecognized fields and collection variants (views, capped)
  • edge_cases.py: Parametrized tests for collection name edge cases and compactionTokens content edge cases
  • smoke test: Fixed to use assertFailureCode instead of checking error message content

What was tested

All 36 tests pass against MongoDB:

  • BSON type exhaustiveness for compactionTokens field
  • Wire-protocol namespace validation (§19 representative case)
  • Non-encrypted collection rejection (core behavior)
  • Collection type variants (view, capped)
  • Unrecognized field rejection
  • Collection name edge cases (system prefix, dots, dollar)
  • CompactionTokens document content edge cases

…tion

- Add NOT_ENCRYPTED_COLLECTION_ERROR constant to error_codes.py
- Use bson_type_validator for exhaustive compactionTokens type rejection
- Use CommandTestCase with pytest_params for parametrized core/error/edge tests
- Fix smoke test to use assertFailureCode (no message content checks)
- Remove redundant response structure standalone test

Signed-off-by: Ian Forster <forstaia@amazon.com>
@imforster imforster requested a review from a team as a code owner June 18, 2026 21:15
@imforster imforster changed the title Add compactStructuredEncryptionData tests with DRY parametrization Add compactStructuredEncryptionData administration command tests Jun 18, 2026
@documentdb-triage-tool documentdb-triage-tool Bot added compatibility test Compatibility test related enhancement New feature or request labels Jun 18, 2026
@documentdb-triage-tool

Copy link
Copy Markdown

🤖 Auto-triaged by documentdb-triage-tool.

Applied: compatibility test, enhancement
Project fields suggested: Component test-coverage · Priority P2 · Effort L · Status Needs Review
Confidence: 0.88 (mixed)

Reasoning

component from path globs (test-coverage, test-framework); effort from diff stats (405+5 LOC, 10 files); LLM: Adds 36 new compatibility test cases for the compactStructuredEncryptionData command across multiple new test files, touching one component with no schema changes.

If a label is wrong, remove it manually and ping @patty-chow so the rules can be tuned. The bot will not re-label items that already have component labels.

@eerxuan eerxuan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

§16 happy-path coverage — verified reachable on mongo-replset

Nice, clean DRY structure throughout (good reuse of bson_type_validator, logical file split, and the smoke-test fix to assertFailureCode). One gap worth addressing before merge.

Every test here asserts a rejection path. Per TEST_COVERAGE.md §16, a collection command also needs its success path — "primary operation succeeds and returns expected response fields." I checked whether that
path is actually reachable, since on standalone it isn't (creating a QE collection fails with 6346402 "Encrypted collections are not supported on standalone", so the all-rejection strategy is correct for that
target).

It is reachable on the mongo-replset target the harness already supports. Tested against mongo:8.2.4:

// QE collection created server-side via create + encryptedFields (UUID keyId)
db.runCommand({
compactStructuredEncryptionData: "enc",
compactionTokens: { ssn: }
})
// →
{ stats: { ecoc: { read: 0, deleted: 0 },
esc: { read: 0, inserted: 0, updated: 0, deleted: 0 } },
ok: 1.0 }

Two things this surfaces:

  1. Success path is testable and returns a documented stats.{esc,ecoc} shape — assertable with assertProperties.
  2. The current compactionTokens "content edge case" tests (null_token_value, empty_string_key, dot_notation_key, nested_document_value) all short-circuit on 6346807 (not-encrypted) on standalone, so they don't
    actually exercise token parsing. On a QE collection, empty tokens against a queryable field instead returns 7294900 "Compact tokens object is missing Compact token for the encrypted path 'ssn'" — that's the
    genuinely-encrypted analog and the path with real signal.

Suggested additions, gated with @pytest.mark.requires(queryable_encryption=True) (the capability already exists in preconditions.py) so they run on mongo-replset and deselect on standalone:

  • Success: QE collection + valid bindata token → assert ok: 1.0 and the stats.esc/stats.ecoc structure.
  • Missing-token (7294900): QE collection with a queryable field + empty compactionTokens.

Notes for whoever picks this up:

  • Server-side create with encryptedFields (keyId as UUID Binary subtype 4) is enough — full client-side auto-encryption (pymongo[encryption]) is not needed for the command path.
  • These must be replica-set gated; QE collection creation itself fails on standalone (6346402).
  • I don't see a QE TargetCollection fixture yet — that small enabling piece is the prerequisite. If adding it is out of scope here, a tracked follow-up referencing this comment is fine so the gap is intentional
    rather than invisible.

Everything else looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

compatibility test Compatibility test related enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants