amberframework / granite

ORM Model with Adapters for mysql, pg, sqlite in the Crystal Language.
MIT License
296 stars 87 forks source link

Add reload method #491

Closed stufro closed 11 months ago

stufro commented 11 months ago

Resolves #485

crimson-knight commented 11 months ago

This is basically an alias of find!, and the test example shows that it's meant to be used when you modify the record outside of the object. Other than the convenience of using .reload instead of Model.find!(), what's the benefit of adding this method?

Do you have a use case?

stufro commented 11 months ago

We have 2 use cases for it.

The first is reloading a record in specs:

person = Person.create
PersonUpdater.run
person.reload.field.should eq "new value"

The second is in a distributed system when running more than 1 instance of the same service, you might want to reload the record in case another instance has updated the db record since it was loaded into memory.

So yeah I agree it is just a convenience method, happy to go with your judgment 😊

a-alhusaini commented 11 months ago

Might I suggest renaming it to sync instead of reload?

Or maybe resync.. since you're syncronizing with the database.. reload is an ok term but it is more imperative than declarative..

I think resync/sync are better names but I think we can come up with something better if we thought about it.

crimson-knight commented 11 months ago

I think this would be helpful in the test suite, so let's wrap this in a macro that checks for the Spec module before it's included.

Can you also add the documentation to the code comments? Now that we have the crystaldoc.info I want to ensure we're writing docs that the community at large can consume in as many places as possible.

stufro commented 11 months ago

Sure thing all done. 1 questions about the docs, when running crystal docs the reload method doesn't appear I'm assuming because the condition in the macro isn't met. Do you know a good way to deal with this?