On Wed, Feb 4, 2015 at 7:45 PM, Tim Williams <[email protected]> wrote:

> I've been giving the new record filtering stuff a spin today and I've
> found bits of it non-obvious.  Mainly with having both o.a.b.user.User
> and o.a.b.thrift.generated.User and setUser and UserContext.setUser.
> It turns out the UserContext gives the behavior I expected
> (implementing a BlurFilterServer) but it's not clear why we need the
> seemingly duplicitous classes/services around?  From a user's
> perspective, btw, the static setter approach feels uncomfortable.  I
> know, "uncomfortable" isn't very constructive - I'm hoping to offer
> that when my question is answered:)
>

Sorry forgot to respond to the first bit of your message.

Java User vs Thrift User, I think there to main reasons.

1. Java User is an immutable object.  (It should be anyway, I realized that
the attribute are not setup to be immutable.)
2. The blur-util project houses the thread pool implementations for Blur
and they are setup to automatically propagate the User object across
threads.  The blur-util can not depend on the generated User in the
blur-thrift project.  Also it didn't feel right to have thrift generated
classes littered in those classes.

The client does have to UserContext.setUser() api we could make it easier
to use.  The real problem is that the normal way in Java to get a client is:

Iface client = BlurClient.getClient(...);

This client actually will call to controllers randomly, so if setUser is
called and then the actual call the client wants to make as that user is
called on another controller.  Not only is the real call made without the
user set correctly in the server session, the first call set a user in the
wrong session potential creating a security issue.

The BlurContext object is read by the BlurClientManager before each real
call and is set regardless if the user changed anything or not.

https://github.com/apache/incubator-blur/blob/master/blur-thrift/src/main/java/org/apache/blur/thrift/BlurClientManager.java#L246

We could (not sure if this a good idea or not) intercept the
Iface.setUser() call from the iface returned from BlurClient and call the
UserContext for the user.  However this won't solve the larger problem,
reseting the user state.

// Member variable
private Iface client = BlurClient.getClient(...);

public void doSomething1(...) {
  client.setUser(user1);
  BlurResults results = client.query(table,query);
  ...
}

public void doSomething2(...) {
  BlurResults results = client.fetchData(table, ...);
  ...
}

// On the same thread
doSomething1();
doSomething2();

The fetch will be called the user1 still set.  This might be not too bad in
a single request, however if this is on a web server with thread pools the
user state will remain across threads which is too dangerous in my opinion.

Aaron


>
> Thanks,
> --tim
>

Reply via email to