Closed rowanmiller closed 2 years ago
@rowanmiller we discussed with @bricelam and @ajcvickers today and we would like to use the core metadata API directly for this (see #1484). We would like move this one to the backlog.
@divega we need to work out how that plays with blocking shadow state entities since the model snapshot can't reference the actual CLR types.
@rowanmiller Do you mean that using the core metadata will make blocking shadow state entities harder?
@divega I meant that the model snapshot needs to be 100% shadow state since it can't reference the model classes. We were planning to block having entities that are 100% shadow state - but perhaps we just block this from the model builder API and allow it in core metadata (though we may want to see if there are places we should detect ad throw in the stack too if we detect a model with shadow entities).
@rowanmiller Ok. I believe that could be done regardless of how the model snapshot is represented. E.g. it could be done in the validation step described in #1205 or potentially in a separate validation step we would only perform while loading the model with the purpose of executing CRUD operations. I don't think the model snapshot would need to be validated in either way.
@rowanmiller I think what you said about blocking it only in the model builder API sounds good too (perhaps that is the same as saying we block in model validation). BTW disabling shadow state entries is tracked in #748.
@rowanmiller @divega I don't think using the core APIs will make it harder to block use of shadow state entities. The idea that has been mentioned most often is to have a check when the context instance is created. That is, you could build a model with shadow state entities, but if you tried to create a session for it, then we would throw. However, I think just removing the ModelBuilder API might be even better since people won't even be led down a path of trying to create them..
@ajcvickers Both sound good to me, remove it from top level API and then have a simple check when you create context.
Sound good.
@divega Is there anything left do to for this issue, or is it all tracked by #1484 and #748 now?
I am ok with closing it, but perhaps we could leave it open in the backlog to consider doing it in the future @rowanmiller @ajcvickers @anpete what do you think?
If we don't have a specific reason to do it now I would close it
Currently we are generating code that provides a shadow state equivalent of the CLR based model. We may need to improve the metadata API/model to properly handle the requirements of a snapshot... or move to another format (possibly JSON).