On 8 Jan 2013, at 10:08 AM, Gunnar Morling <gunnar(a)hibernate.org> wrote:
I'm not sure whether it has been considered before, but maybe we
could unify the methods and work with a parameter object as a middle ground:
It has been. I suggested before to combine the methods. I think it is a good approach, but
Sanne thought it is bad that he user has to deal with
null values.
public interface ShardIdentifierProvider {
void initialize(Properties properties, BuildContext buildContext);
Set<String>
getShardIdentifiersForEntity(EntityShardIdentifierRetrievalContext retrievalContext);
Set<String>
getShardIdentifiersForQuery(QueryShardIdentifierRetrievalContext retrievalContext);
Set<String> getAllShardIdentifiers();
}
It is a reasonable approach. Better then what we have. I am drawn between this one and the
additional interface. This approach has the advantage
that as you say we can modify the internas of the context instances.
EntityShardIdentifierRetrievalContext would provide all parameters
usable for shard determination, clearly stating that "document" is not available
in cases such as deleting.
Such a parameter object would allow for adding more options in the future in a compatible
manner, and also the method names read quite nicely and symmetrically
(I share Hardy's concerns about the asymmetry of getShardIdentifier() vs.
getShardIdentifiersForDeletion()).
All correct imo.
The disadvantage of this scheme is that a set needs to be returned
also for the case of returning a single identifier during insert/update, which might
render the approach not feasible.
OTOH I'm wondering why a set needs to be returned for the delete case, your example
also returns exactly one identifier?
See the comments I made on the pull requests. I also don't think the deletion case
should return a set. Either one knows the shard it and returns it or one does not know and
you have
to return null. In the latter case it is up to us to apply the default strategy. IMO it is
wrong to say to the user "if you don't know just return all shard ids". It
takes away the option from us
to distinguish between these two cases.
--Hardy