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

Improve Version constructor logic #146

Closed
joepio opened this issue Jul 5, 2021 · 0 comments
Closed

Improve Version constructor logic #146

joepio opened this issue Jul 5, 2021 · 0 comments

Comments

@joepio
Copy link
Member

joepio commented Jul 5, 2021

Commits can be played back in order to construct a specific version of a Resource. The current versioning implementation #42 does this fine, but the current approach has some major flaws.

First, I tried to instantiate a new empty Store, and apply all commits over there. The downside of this, is that it is very slow, because every .set action with a property unfamiliar to that store, will result in a fetch - the store needs to know that property in order to actually set the data.

Then, I tried to temporarily remove the resource from the store, construct it from commits, return the constructed resource, and put back a clone of the original resource. This is way quicker, but it is kind of dirty. Firstly, if anything goes wrong in between - the data is gone or damaged. Secondly, if someone fetches the edited resource while the commits are being applied, we could very well get a wrong representation of that resource, and this turns out to happen every time we try to render the /all-versions endpoint for some resource. Not good.

So I tried to have an optional parameter to the commit function for a guest_store, which could be used as a source for the properties, but Rust doesn't like using two different impl trait inputs in one function.

Maybe there is another aproach. Perhaps the commit function should return also returns the resource that is to be added to the store, instead of directly adding it to the store by itself. We could have a more detailed commit_extra function... Or maybe extract the set and remove logic to a separate method in Commit. Yeah, let's do that.

joepio added a commit that referenced this issue Jul 5, 2021
@joepio joepio closed this as completed Jul 5, 2021
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

No branches or pull requests

1 participant