incubator-esme-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Pollak <feeder.of.the.be...@gmail.com>
Subject Re: The Pool stuff is good to go
Date Fri, 19 Jun 2009 20:10:09 GMT
On Fri, Jun 19, 2009 at 12:30 PM, Vassil Dichev <vdichev@apache.org> wrote:

> > 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.


It's easier than that... all models have an "addlQueryParams" instance
variable.  As a wrapper, define it to limit access to the current pool.  If
you've got questions on how to do it, ping me and we can work together on
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
>



-- 
Lift, the simply functional web framework http://liftweb.net
Beginning Scala http://www.apress.com/book/view/1430219890
Follow me: http://twitter.com/dpp
Git some: http://github.com/dpp

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message