Yicong-Huang commented on code in PR #7057:
URL: https://github.com/apache/texera/pull/7057#discussion_r3678290338
##########
amber/src/main/scala/org/apache/texera/web/ServletAwareConfigurator.scala:
##########
@@ -97,19 +90,11 @@ class ServletAwareConfigurator extends
ServerEndpointConfig.Configurator with La
val claims = jwtConsumer.process(token).getJwtClaims
config.getUserProperties.put(
classOf[User].getName,
- new User(
- claims.getClaimValue("userId").asInstanceOf[Long].toInt,
- claims.getSubject,
-
String.valueOf(claims.getClaimValue("email").asInstanceOf[String]),
- null,
- null,
- null,
- null,
- null,
- null,
- null,
- null
- )
+ new User().tap { user =>
+
user.setUid(claims.getClaimValue("userId").asInstanceOf[Long].toInt)
+ user.setName(claims.getSubject)
+
user.setEmail(claims.getClaimValue("email").asInstanceOf[String])
Review Comment:
The old positional call wrapped this in `String.valueOf(...)`, which for a
null `email` claim stored the literal string `"null"` (the `String`-typed arg
binds `String.valueOf(Object)`). Dropping the wrapper stores `null` instead.
For a present claim the two are identical — the only difference is the
null-email edge, and the new behavior is arguably more correct and now
consistent with the header branch and `JwtParser`, which never wrapped email.
Just confirming it's intended, since this is labeled a pure refactor. (Codecov
also flags this as the one uncovered patch line, so the null path isn't
exercised by a test.)
--
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]
