James 4099 Configurable path delimiter

[j-hellenberg]

James-4099

Add a configuration switch for adjusting the default mailbox folder delimiter.

Currently allowed (because unit-tested) values are: dot (will use '.' as delimiter), slash (will use '/' as delimiter), pipe ('|'), comma (','), colon (':'), semicolon (';').

Currently explicitly left-out values are: '#' (clashes with namespace prefix character), '\' (anticipated some problems with the prefixedRegex matching and because it is the escaping character, it can generally be a bit more annoying to deal with in strings).

This feature is opt in - the default configuration will still retain the current default delimiter (dot).

Notes to reviewer:

• Unfortunately, git becomes pretty confused when indentation of a large code chunk is changed. For this reasons, it might make sense to look at some commits individually - I specifically separated refactoring and correctly intending the MailPathTests and PrefixedRegexTests into their own commits to improve diff readability.
• There may be more places in the code where '.' is used as a hard-coded constant. Is this an issue? Obviously, the dot used in Domain.java or MailAddress.java is independent of the mailbox folder delimiter, but the FileIntoAction, for example, also has a hierarchy delimiter.

[chibenwa]

FileIntoAction follows sieve convention to use / as a folder delimiter and needs a translation to the actual deleimiter. Email, i know...

[j-hellenberg]

The old version of the relax-mode tests essentially asked "If I do nothing, does it throw an exception?" 😁 Obviously, that sort of mistake is easy to overlook as the resulting test trivially passes, but I still found it a bit funny

[chibenwa]

That's what happens when TDD is overlooked I believe!

[chibenwa]

It looks overall good to me.

Thanks for the quality contribution.

I did took the time to review what Cyrus does. It offers either . or / as a delimiter. So I am OK with the implemented behavious.

Please clearly state in the documentation AND in the configuration comment that this option is not safe to change once you have data and will result in loss of the folder hierarchy.

[j-hellenberg]

Please clearly state in the documentation AND in the configuration comment that this option is not safe to change once you have data and will result in loss of the folder hierarchy.

Currently, I'm writing everywhere WARNING: This value should only be changed when setting up a new deployment. Changing the parameter for an existing deployments will likely lead to failure of some system components, as occurrences of old the delimiter will still be present in the database/data store. Would you like that comment to be reworded in some way? Or do you see a place where I am missing the comment?

[j-hellenberg]

FileIntoAction follows sieve convention to use / as a folder delimiter and needs a translation to the actual deleimiter. Email, i know...

I saw that the execute method does destinationMailbox.replace(FileIntoAction.HIERARCHY_DELIMITER, '/') (destinationMailbox.replace('.', '/'), in other words).

So, to be clear: If the MailboxConstants.FOLDER_DELIMITER is anything except '/', this line needs to be destinationMailbox.replace(MailboxConstants.FOLDER_DELIMITER, '/') (and it works fine if delimiter is already /, in which case this is a noop)?

[chibenwa]

So, to be clear: If the MailboxConstants.FOLDER_DELIMITER is anything except '/', this line needs to be destinationMailbox.replace(MailboxConstants.FOLDER_DELIMITER, '/') (and it works fine if delimiter is already /, in which case this is a noop

Yes!

[j-hellenberg]

So, to be clear: If the MailboxConstants.FOLDER_DELIMITER is anything except '/', this line needs to be destinationMailbox.replace(MailboxConstants.FOLDER_DELIMITER, '/') (and it works fine if delimiter is already /, in which case this is a noop

Yes!

Okay, so I added that. Do you know of any more locations where it would be critical to honor the new delimiter? (I did not find any, but I'm not sure that means much)

[vttranlina]

+1, When I read RFC documents, It tend to use / more frequently as an example.