chibenwa commented on PR #2405:
URL: https://github.com/apache/james-project/pull/2405#issuecomment-2352160764
Using functionnal constructs we can get it slightly cleaner:
```
mail.getMaybeSender().map(usersrepository::getUsernameForMailAddress).orElse(null);
```
Which is slightly better (use UsersRepository to convert MailAddress to
Username!)
> What do you suggest ? I could adapt resolveRight() to accept
Optional.empty() instead of null, but is it my job to do it ?
I Am not fan either to be honest: null as a parameter for `resolveRights`
usage being working seems mostly accidental...
- Do the current code base currently uses "null" to denote absence of user
or "anybody"?
- Do the current code base actively test this specific value and specifies
clearly the overall behaviour?
As none of these answer is negative we likely cannot rely on this 'null'
parameter.
Once again 'p' implies resolving rights as part of the delivery context
which is by essence different of applying rights directly linked to user
actions. Hence having 'SubAddressing feature bridging the gap seems reasonable
to me.
In pseudo code this is what I would write:
```
MailboxACL.Rfc4314Rights rights =
mail.getMaybeSender().map(usersrepository::getUsernameForMailAddress)
.map(resolveUserRights)
.orElseGet(getAnyBodyRights); // read directly the ACL for null sender
```
What I also really do not like is the third parameter being an untyped
String and not a stronger type.... Like `Username`. We *could* get one commit
to address this.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
