I have a method which extracts the logged-in user details from a SpringSecurityContext
object.
I have read that only utility methods (which does certain calculations) should be static
.
Here is my method; it doesn't seem to be a utility method but I don't find any reason why I shouldn't make it static
as I am using it in multiple beans.
public static int getSignedUpUser()
{
final SecurityContext ctx = SecurityContextHolder.getContext();
if(ctx != null)
{
final Authentication auth = ctx.getAuthentication();
if(auth != null)
{
final Object principal = auth.getPrincipal();
if(principal instanceof AUser)
{
final AUser au = (AUser)principal;
return au.getId();
}
}
}
return 0;
}
}
1 Answer 1
This is a utility method. There's nothing wrong with it being static. Its readability would benefit greatly from (a) using guard clauses instead of having multiple nesting levels, (b) using java bracket style, and (c) using java spacing. You might also want to add logging to indicate why there's no signed-in user, which is presumably a WARN or ERROR, but should at least be a DEBUG.
public static int getSignedUpUser() {
final SecurityContext ctx = SecurityContextHolder.getContext();
if (ctx == null) {
LOGGER.debug("No security context available");
return 0;
}
final Authentication auth = ctx.getAuthentication();
if (auth == null) {
LOGGER.debug("No authentication available in security context {}", ctx);
return 0;
}
final Object principal = auth.getPrincipal();
if (!(principal instanceof AUser) {
LOGGER.warn("Principal {} is not an instance of AUser!", principal);
return 0;
}
final AUser au = (AUser)principal;
return au.getId();
}