Yeah, you kind of got to the same conclusion as I did... i.e. not sure what to do. If we add the EntityDescriptor, there should be some semantics on what it means, not arbitrarily from ES to ES.
I think we just keep it as it is. On Mon, May 8, 2017 at 6:19 PM, Paul Merlin <[email protected]> wrote: > Hey Niclas, > > Niclas Hedhman a écrit : > > Peddling with this JOOQ ES (or whatever it ends up being called), I found > > an inconsistency in the EntityStore SPI. > > > > > > public interface EntityStoreUnitOfWork > > { > > EntityState newEntityState( EntityReference anIdentity, > > EntityDescriptor entityDescriptor ) > > throws EntityStoreException; > > > > EntityState entityStateOf( ModuleDescriptor module, > > EntityReference anIdentity ) > > throws EntityStoreException, EntityNotFoundException; > > > > String versionOf( EntityReference anIdentity ) throws > EntityStoreException; > > > > } > > > > > > It is not consistent to pass the EntityDescriptor in newEntityState() but > > not in entityStateOf(). In the latter case, it is expected that the > > EntityStore have saved away the type, and able to look it up from the > > Module, when the caller already have the EntityDescriptor readily > available. > > > > Can anyone think of a case where it is not better to pass the > > EntityDescriptor from the caller? > > > > It might be related to looking up via a super type, and if then saving, > > then there is a chance that the "old fields" are no long present. And to > > avoid that, push the whole "what is the lowest subclass of this identity" > > to the entitystore. > > > > I think my question is, what is the proper semantics around entity > > subclasses? Is it really the "saved type" that is the deserialized type, > or > > is it the "requested type" that should be the deserialized type? > > > > I am not sure either way, and would like to hear more arguments one way > or > > the other. > > Yeah, agreed that it doesn't feel consistent at first sight. > > BUT, > When creating an entity we don't have anything else than its > EntityDescriptor. > When saving the entity its `type` is set to the main type of its > EntityDescriptor. > Then, when fetching an entity the ES will load the whole state and use > the stored `type` to lookup the corresponding EntityDescriptor, and fail > with a NoSuchEntityTypeException if not found. > > As you wrote, this means that you can fetch an Entity using a super > type, the loaded state will however contain all the data described by > the EntityDescriptor used on creation. API-wise you'll only be able to > mutate the entity using the "requested type" but under the hood, the > whole state is kept. > > I think it makes sense. Especially when thinking about key/value backed > EntityStores where the whole state is a "blob". > > If we change that SPI so that `entityStateOf(..)` takes an > EntityDescriptor then it would mean that EntityStores could have the > opportunity to only load a "view" of the entity and have to handle > supplementary fields non destructively. As I see it adds a feature to > the SPI and a new responsibility for EntityStores. > > Key/Value stores could then still do as they do today, possibly > validating that the saved EntityDescriptor types include all of the > requested EntityDescriptor types. Or some sort of validation. It's an > extra responsibility for the EntityStores. > > More evolved EntityStores could then only load "views" of entities, not > the whole saved state. But they also should be able to update state non > destructively. > > I'd say it's a tradeoff between capabilities and implicit > responsibilities for implementations. I'm not against adding the feature > to the SPI but then we should at least properly document it so > asumptions are crystal clear. > > Does that help? > > Cheers > > /Paul > > > -- Niclas Hedhman, Software Developer http://polygene.apache.org - New Energy for Java
