Skip to content

feat(value): introduce Set storage abstraction#740

Open
anakrish wants to merge 1 commit into
microsoft:mainfrom
anakrish:storage-abstraction-set-foundation
Open

feat(value): introduce Set storage abstraction#740
anakrish wants to merge 1 commit into
microsoft:mainfrom
anakrish:storage-abstraction-set-foundation

Conversation

@anakrish
Copy link
Copy Markdown
Collaborator

@anakrish anakrish commented Jun 4, 2026

Adds an opaque Set newtype paralleling Object, living under src/value/set/ with the same module structure as src/value/object/ (mod.rs / iter.rs / serde.rs).

Set wraps BTreeSet<Value> today but exposes only a curated surface: contains, insert, remove, iter / iter_sorted, cursor (resumable), is_subset, intersection, difference, serde, and a hand-written Ord. The cursor type is re-exported behind the rvm feature so the follow-up IterationState::Set swap can land additively.

To free the Set name for the new public type, the crate-internal BTreeSet as Set / HashSet as Set alias in lib.rs is renamed to MapSet. Only one in-tree consumer existed (compiled_policy::CompiledPolicy::rule_paths) and is updated in lockstep.

Value::Set is unchanged in this PR (still Rc<BTreeSet<Value>>); the payload swap and call-site migration ship in a follow-up PR.

Tests live in src/value/tests.rs alongside the Object tests.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Introduces a new opaque value::Set container (mirroring value::Object) to abstract set storage behind a curated API, while freeing the Set identifier by renaming the internal lib.rs collection alias to MapSet.

Changes:

  • Added src/value/set/ (Set type, iterator wrappers, and serde support) and re-exported Set (and SetCursor behind rvm) from src/value/mod.rs.
  • Updated the single in-tree consumer of the old crate-internal Set alias (CompiledPolicyData::rule_paths) to use MapSet.
  • Added unit tests for Set behavior and serde round-trips; added docs/value/set.md describing the design.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/value/tests.rs Adds test coverage for the new Set API (iteration, algebra, cursor, serde, COW behavior via Value::Set).
src/value/set/mod.rs Implements the new Set abstraction, including cursor iteration and set algebra helpers.
src/value/set/iter.rs Adds opaque iterator newtypes and IntoIterator impls for Set and &Set.
src/value/set/serde.rs Implements Serialize/Deserialize for Set, with per-element memory-limit checks during deserialization.
src/value/mod.rs Wires the new set module and re-exports Set (and SetCursor behind rvm).
src/lib.rs Renames the crate-internal Set collection alias to MapSet to avoid colliding with the new public Set type.
src/compiled_policy.rs Updates rule_paths storage type from Set<String> to MapSet<String>.
docs/value/set.md Documents the new Set abstraction and intended backend-swappability.

Comment thread src/value/set/serde.rs Outdated
Comment thread src/value/set/mod.rs
Comment thread docs/value/set.md
Comment thread src/value/set/mod.rs
@anakrish
Copy link
Copy Markdown
Collaborator Author

anakrish commented Jun 4, 2026

@copilot review this PR using all the review skills in this repo. Use a separate agent for each skill.

@anakrish anakrish force-pushed the storage-abstraction-set-foundation branch from 2dd9c43 to 39fff42 Compare June 4, 2026 19:37
@anakrish anakrish requested a review from Copilot June 4, 2026 19:39
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

@anakrish anakrish requested review from dekomissMSFT and dpokluda June 5, 2026 18:12
@anakrish anakrish marked this pull request as ready for review June 5, 2026 23:04
Add an opaque `Set` newtype paralleling `Object`, living under
`src/value/set/` with the same module structure (`mod.rs` /
`iter.rs` / `serde.rs`). `Set` wraps `BTreeSet<Value>` today but
exposes only a curated surface: `contains`, `insert`, `remove`,
`iter`, `iter_sorted`, `cursor` (resumable), `is_subset`,
`intersection`, `difference`, serde, and a hand-written `Ord`.
The cursor types are re-exported behind the `rvm` feature so the
follow-up `IterationState::Set` swap can land additively.

To free the `Set` name for the new public type, the crate-internal
`BTreeSet as Set` / `HashSet as Set` aliases in `lib.rs` are
renamed to `MapSet`. All in-tree consumers of the old alias are
updated in lockstep.

`Value::Set` is unchanged in this commit (still wraps
`Rc<BTreeSet<Value>>`); the payload swap and call-site migration
ship in the next PR.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@anakrish anakrish force-pushed the storage-abstraction-set-foundation branch from 39fff42 to 923da90 Compare June 6, 2026 12:58
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