A little bit back Mike added a fast-path for Key construction for keys with
one Comparable. Keys with multiple values would still validate the order
every time. This was necessary even when using the generated factory
because Class' getFields() does not guarantee any order. In practice,
the Sun JVM seems to return them in declared order, but apparently other
JVMs may not. The arguments to getKey() are determined when 'genrecord'
is run, but the field order used during runtime is computed at runtime.
This change generates code that stashes the key fields in an order
deteremined at the time 'genrecord' is run and that is guaranteed to
match the argument order to each PersistentRecord subclass' getKey().
Thus, getKey() can now directly construct a key with minimal overhead.
The field order is still computed at runtime as a fallback for
PersistentRecord classes that haven't yet been updated.
The ordering of getFields() is still being relied upon at 'genrecord'
time, but Mike may introduce a patch that will guarantee that ordering
matches declaration order. In the meantime, it's no worse than before
and if somehow someone does run 'genrecord' with an alternate JVM
they should immediately notice the discrepency in their diffs.
The Key constructor utilized is still public because it is used by
DepotUtil. A nice change would be to make it package-private.
However, I started making HSQL work a long time ago so that we could have real
unit tests that did real meaningful things, and Zell was kind enough to finish
making HSQL work, so we should be able to do some real end-to-end testing. I
just need to set up the necessary scaffolding. On the list!
Note one potentially consequential diff in DepotRepository:
- return keys.isEmpty() ? Collections.<T>emptyList() :
- _ctx.invoke(new FindAllQuery.WithKeys<T>(_ctx, keys));
+ return _ctx.invoke(new FindAllQuery.WithKeys<T>(_ctx, keys));
This should not actually impact anything. FindAllQuery.WithKeys first checks
for keys that are cached, coping with a potentially empty set (there will be
none), then if there are keys that remain to be loaded from the database, it
loads them. The first check will NOOP as will the second check. So this should
just be a slightly more circuitous path to getting back an empty collection. It
should not result in a zero row matching query being sent to the database.
With that in mind, it seemed inelegant to prematurely optmize the return of an
empty collection.
Sequence an interface.
We're going to change all of the places in Depot where we take a Collection as
an argument and take an Iterable instead. We invariably just turn the
Collection to an array, and we can just as well do that with an Iterable and be
more flexible about what we accept. Iterables.toArray has a fast path for
Iterable instances that also implement Collection, so we make SeqImpl implement
Collection under the hood even though we don't advertise that in the Sequence
interface because we don't want people using these as collections.
What we really want is something in between Iterable and Collection that knows
its size but otherwise does not allow mutation or containment checks.
Considered adding a note that toArray() is generally not safe, but the
Collections run through here in normal depot usage will be safe to use.
Check out the implementation of AbstractCollection's toArray(T[]).
public and I should not force everyone who wants to use Depot to route all
database accesses through custom public methods that hide all of the underlying
Depot business behind application specific APIs.
I still think this is the best way to use Depot, but I acknowledge that there
are certain valid use cases (like generic code that operates on records in
arbitrary repositories) which are precluded by this scheme. So instead I'll
just strongly suggest that Depot be used in the way I think it should, and
frown sternly at anyone who deviates from The Way.
Ray Greenwell