Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a way to update an entity inside an Entities object #1479

Open
1 of 2 tasks
a0js opened this issue Feb 18, 2025 · 1 comment
Open
1 of 2 tasks

Add a way to update an entity inside an Entities object #1479

a0js opened this issue Feb 18, 2025 · 1 comment
Labels
feature-request This issue requets a substantial new feature pending-review A Cedar maintainer has looked at this, but believes it needs review by more of the core team

Comments

@a0js
Copy link

a0js commented Feb 18, 2025

Category

User level API features/changes

Describe the feature you'd like to request

Right now there is no way to update an entity inside an Entities object. For instance, if in my app, a user was assigned to a user group (a parent relation in cedar world), I'd like to be able to set an individual entity and have the relations updated:

let entities = Entities::from_entities(entities_from_db, schema)?;

// update
let update_or_insert_operation = entities.set(entity, schema)?;

// OR
entities.add_entities([entity], schema); // overwrite rather than error on duplicate.

Describe alternatives you've considered

This is what I'm doing currently:

let mut new_entities: Vec<_> = entities
        .iter()
        .filter(|cur_entity| entity.uid() != cur_entity.uid())
        .cloned()
        .collect();

new_entities.push(entity);
entities = Entities::from_entities(new_entities.into_iter(), schema)?;

It just seems unnecessary to clone the entire entities object, just to update one entity, though I am unfamiliar with the deeper workings of the cedar_policy_core module.

Additional context

No response

Is this something that you'd be interested in working on?

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change
@a0js a0js added feature-request This issue requets a substantial new feature pending-triage The cedar maintainers haven't looked at this yet. Automicaly added to all new issues. labels Feb 18, 2025
@a0js
Copy link
Author

a0js commented Feb 18, 2025

I did see that remove_entities() is getting added, which looks great. But I don't want to use that either as I'd be recalculating the graph relations on remove and add. Looking at some of the internal code, I wonder if this can be accomplished by adding an overwrite boolean option to the add_entities() function which would be forwarded all the way down to the update_entity_map() or maybe adding an upsert_entities() function that would call the internal functions with that boolean flag, so the add_entities doesn't introduce a breaking change.

Another option I see is, adding an upsert_entities() function that runs the remove_entities() first with tc_computation set to AssumeAlreadyComputed, and then run the add_entities() as normal:

// api.rs

    pub fn upsert_entities(
        self,
        entities: impl IntoIterator<Item = Entity>,
        schema: Option<&Schema>,
    ) -> Result<Self, EntitiesError> {
        let entities: Vec<_> = entities.into_iter().collect();
    
        let removed = self.0.remove_entities(
            entities.iter().map(|e| e.uid().0),
            cedar_policy_core::entities::TCComputation::AssumeAlreadyComputed
        )?;
    
        Ok(Self(
            removed.add_entities(
                entities.into_iter().map(|e| Arc::new(e.0)), 
                schema.map(|s| cedar_policy_validator::CoreSchema::new(&s.0))
                    .as_ref(),
                cedar_policy_core::entities::TCComputation::ComputeNow,
                Extensions::all_available(),
            )?
        ))
    }

@a0js a0js mentioned this issue Feb 18, 2025
4 tasks
@shaobo-he-aws shaobo-he-aws added pending-review A Cedar maintainer has looked at this, but believes it needs review by more of the core team and removed pending-triage The cedar maintainers haven't looked at this yet. Automicaly added to all new issues. labels Feb 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request This issue requets a substantial new feature pending-review A Cedar maintainer has looked at this, but believes it needs review by more of the core team
Projects
None yet
Development

No branches or pull requests

2 participants