esme-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vassil Dichev <vdic...@apache.org>
Subject Re: The Pool stuff is good to go
Date Fri, 19 Jun 2009 19:30:47 GMT
> I've looked at the pool stuff.  Vassil did a great job with it.  I made a

Thank you!

> very minor performance enhancement (using Set instead of List which means
> O(log n) rather than O(n) for access checking).

Good idea. I have some concerns about making a new query for each pool
a user can write to just to get its name, but it might be early to
optimize, and we can certainly do it later on.

> Rock and roll and if it's got the votes (which I think it does), I say roll
> it on into trunk.

Great, I will proceed with the merge right away.

@David, I want your advice on one more issue. There are many
occurrences in ESME where a find* method is called on Message. These
calls would retrieve also messages from pools the user is not supposed
to see. What do you think is a better solution:

-I'm toying with the idea of overriding findMapDb (since all calls to
findAll and findMap eventually call it). I'm trying to insert an SQL
fragment, which would restrict results only to the allowed pools. This
is a single point of control, so should be easier to maintain. OTOH it
could be less transparent and harder to debug if you don't expect it.

-the standard solution of changing all instances of findAll/findMap is
more easy to understand, but harder to maintain- one must make sure no
find* call is forgotten.

Overall, I'm leaning towards overriding the single findMapDb. It's
similar to aspects: it might be more difficult to debug and reason
about, but it's cleaner and separates the concern. The find* methods
are part of the model API and potential new ESME committers could
forget to restrict the call to only return messages from appropriate
pools.

Everyone: relax, this will not delay merging, we will resolve the
issue afterwards.

Cheers,
Vassil

Mime
View raw message