node-ottoman icon indicating copy to clipboard operation
node-ottoman copied to clipboard

Check the model type when using `findById`

Open xavierchow opened this issue 3 years ago • 4 comments

I'm not quite sure if it's a bug, but the behavior of ottoman doesn't match my assumption.

I have a use case that uses uuid without prefix as the key of a document, and multiple models reside in a single bucket/scope/collection. The problem is that when applying an Id of model A to ModelB.findById, it just returns the document of model A.

I know it's not expected to apply the wrong Id to the model.findById, but it would be ideal if it can return a DocumentNotFoundError instead of a data from other models.

Attaching a minimum reproducible code as follows, https://gist.github.com/xavierchow/a995771877a9bac3ff56e192ae3aef2f

I'm wondering if an improvement with checking the model type for findById makes sense?

xavierchow avatar Apr 24 '22 06:04 xavierchow

@xavierchow probably a bug like you had mentioned. @gsi-alejandro can you please review and provide a feedback ?

AV25242 avatar Apr 25 '22 15:04 AV25242

hi @xavierchow

We need to discuss it. This issue will never arise while using scopes and collections.

The problem is: to ensure this now we have the metadata _type, this way we know if the document belongs to the model that requests it maybe in the future, we have to remove permanently or by a flag option the _type, then this implementation will not work anymore.

gsi-alejandro avatar Apr 25 '22 17:04 gsi-alejandro

@gsi-alejandro IMHO, the Ottoman as the ODM maintains the mapping between the Model to the documents in the DB. It's strange that I can use A.findById to get a B document.

Good to know you may have a plan about the metadata _type, but how to distinguish the models (by collection separation or by metadata _type) doesn't block the improvement that brings the concept integrity, what do you think?

xavierchow avatar Apr 26 '22 02:04 xavierchow

Hi @xavierchow, thank you for your feedback.

We will provide a solution for its next release (check for doc _type against the model)

gsi-alejandro avatar May 03 '22 16:05 gsi-alejandro

released on version 2.2.1

gsi-alejandro avatar Oct 18 '22 14:10 gsi-alejandro