A small nitty-gritty thought ...
is it worthwhile swapping the arguments of #entityStateOf, such that all
methods have an entityReference as 1st argument? (another way of
creating a bit more consistency)
/Kent
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;
> }
Den 08-05-2017 kl. 16:34 skrev Niclas Hedhman:
> 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 <paulmer...@apache.org> 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
>>
>>
>>
>
