andrii0lomakin commented on code in PR #3137:
URL: https://github.com/apache/tinkerpop/pull/3137#discussion_r2166354019
##########
gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java:
##########
@@ -233,15 +236,23 @@ protected void evalOpInternal(final Context ctx, final
Supplier<GremlinExecutor>
final GremlinExecutor.LifeCycle lifeCycle =
GremlinExecutor.LifeCycle.build()
.evaluationTimeoutOverride(seto)
- .afterFailure((b,t) -> {
+ .afterFailure((b, t) -> {
graphManager.onQueryError(msg, t);
- if (managedTransactionsForRequest) attemptRollback(msg,
ctx.getGraphManager(), settings.strictTransactionManagement);
+ if (managedTransactionsForRequest)
+ attemptRollback(msg, ctx.getGraphManager(),
settings.strictTransactionManagement);
+ graphManager.afterQueryEnd(msg);
})
.afterTimeout((b, t) -> {
- graphManager.onQueryError(msg, t);
+ graphManager.onQueryError(msg, t);
+ graphManager.afterQueryEnd(msg);
})
.beforeEval(b -> {
- graphManager.beforeQueryStart(msg);
+ AuthenticatedUser user =
ctx.getChannelHandlerContext().channel().attr(StateKey.AUTHENTICATED_USER).get();
+ if (null == user) {
+ // This is expected when using the
AllowAllAuthenticator
+ user = AuthenticatedUser.ANONYMOUS_USER;
+ }
+ graphManager.beforeQueryStart(msg, user);
Review Comment:
@andrii0lomakin Yes, but that is very high level in what scenario? In a
scenario, when does it already exist in the channel as an attribute? Or should
this test cover all cases of authentication? I believe that such a test case
should either exist or be added as a separate change, as that is not the point
of this PR to test the correctness of authentication mechanics. Do you have any
objections?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
