fix: support polymodels with equal names but different roots (#961) #986
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the new version of NDB a breaking requirement/implementation has been added requiring all the polymodel leaves to have unique names. There is no technical limitation for this requirement either on client or server side. So, by the introduction of this requirement we make it unable for the clients with legacy data to retrieve their models written by
appengine.ext.ndb.Interestingly enough, currently there is an unused class property
PolyModel._class_mapwhich is populated by it but is never read. On the other side we havemodel._entity_from_ds_entitythat discards the rest of theclass_keypassed to it, taking just just the last element and looking it as if it were an ordinary model. These two shortcomings are as if calling for each other - an unused property and a client using incomplete data.This PR fixes this inconsistency and re-enables the ability to read and write polymodels with equal names if their class_keys are different. For this purpose the
_class_mapproperty is moved toModelclass so that it can read from it. I avoided introduction of a function for accessing it, because unlikelookup_kindthis function should not raise an exception if the model is not found. Anyhow, it is requested during the review, I will add a wrapper function along with its tests or perform any other changes more persistent to the design of the library.