You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by "ottoka (via GitHub)" <gi...@apache.org> on 2024/02/14 11:27:22 UTC

[PR] JAMES-3991 Vacation handling should be case insensitive [james-project]

ottoka opened a new pull request, #2002:
URL: https://github.com/apache/james-project/pull/2002

   Easiest way to achieve this goal is to always use lower case in the AccountId wrapper. That should fix the issue everywhere.
   Also extended the DAO contracts with tests for case insensitive access.


-- 
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: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


Re: [PR] JAMES-3991 Vacation handling should be case insensitive [james-project]

Posted by "ottoka (via GitHub)" <gi...@apache.org>.
ottoka commented on PR #2002:
URL: https://github.com/apache/james-project/pull/2002#issuecomment-1944032476

   Re your vanished(?) comment: I thought about doing this in the DAOs instead of AccountId, but then you have to do it in multiple places (error prone) and potentially repeated for each storage backend. Thus I decided to go with this centralized solution.


-- 
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: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


Re: [PR] JAMES-3991 Vacation handling should be case insensitive [james-project]

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #2002:
URL: https://github.com/apache/james-project/pull/2002#issuecomment-1944846657

   Yes and after commenting I weighted the pros and cons and decided that putting up a gas factory for this was likely not worth it.


-- 
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: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


Re: [PR] JAMES-3991 Vacation handling should be case insensitive [james-project]

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2002:
URL: https://github.com/apache/james-project/pull/2002#discussion_r1489566000


##########
server/data/data-api/src/main/java/org/apache/james/vacation/api/AccountId.java:
##########
@@ -40,7 +40,7 @@ public static AccountId fromUsername(Username username) {
     private final String identifier;
 
     private AccountId(String identifier) {
-        this.identifier = identifier;
+        this.identifier = identifier.toLowerCase();

Review Comment:
   I would rather see this in the NotificationRegistry if possible.



-- 
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: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


Re: [PR] JAMES-3991 Vacation handling should be case insensitive [james-project]

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2002:
URL: https://github.com/apache/james-project/pull/2002#discussion_r1489566000


##########
server/data/data-api/src/main/java/org/apache/james/vacation/api/AccountId.java:
##########
@@ -40,7 +40,7 @@ public static AccountId fromUsername(Username username) {
     private final String identifier;
 
     private AccountId(String identifier) {
-        this.identifier = identifier;
+        this.identifier = identifier.toLowerCase();

Review Comment:
   I would rather see this in the NotificationRegistry if possible.



-- 
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: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


Re: [PR] JAMES-3991 Vacation handling should be case insensitive [james-project]

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael merged PR #2002:
URL: https://github.com/apache/james-project/pull/2002


-- 
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: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org