Moving it to processor does not hurt, but I don't understand why using the MailboxSession would create a cycle-dependency, Could you explain in detail ?
Bye, Norman 2011/1/7 Wojciech Strzałka <[email protected]>: > > Working on it one by one. > > I have one more problem however. > > In the CreateCommandParser::decode() there delimiter is used. I don't > have an idea where from to take it. > Previously I've tried to get MailboxSession using ImapSession but this > adds dependency from message to processor - what causes cyclic > dependency problem. > The only idea I have about it is to move this logic: > > -------------------------------------- > // [email protected] > // When mailbox name is suffixed with hierarchy separator > // name created must remove tailing delimiter > if (mailboxName.endsWith(Character.toString(delimiter))) { > mailboxName = mailboxName.substring(0, mailboxName.length() -1); > } > -------------------------------------- > > from CreateCommandParser to CreateProcessor. > > wdyt? > > >> Hi there, > >> from a quick review it make sense.. I just notices you only did the >> changes for jpa mailbox yet and also the unit tests are broken.. >> Anyway I think it should work once this is sorted. One other thing I >> noticed while reading the diff and the imap rfc is the possibility of >> having a "NIL" seperator. We currently use a null String for this. >> With char we could use Character.UNASSIGNED for this.. > >> Wdyt ? > >> Bye, >> Norman > > >> 2011/1/5 Wojciech Strzałka <[email protected]>: >>> >>> Could you review the patch attached? It's a little bigger then >>> I'd expected. >>> >>> The central point for delimiter definition is >>> StoreMailboxManager::getDelimiter() which can be overloaded. Default >>> implementation uses MailboxConstants.DEFAULT_DELIMITER ( a '.' ) as a >>> delimiter. >>> All other references to this constant were removed. All other >>> separator definitions also. >>> >>> I'm not 100% sure of changes made to AbstractImapCommandParser. >>> New method decode(...) was introduced with default implementation >>> calling old method (to avoid changing all the CommandParser). Hovewer >>> abstract keyword was not removed from old implementation so the only >>> command implementing the new version (CreateCommandParser) needs to >>> provide old version method (an empty implementation). >>> >>> Don't hesitate to make any change requests or just throw it away if >>> you don't like it. >>> >>> I'm planning to test it a little in the next few days. >>> >>>> Hi there, >>> >>>> comments inside.. >>> >>>> 2011/1/3 Wojciech Strzałka <[email protected]>: >>>>> >>>>> I'm trying to implement it and I have a few questions: >>>>> >>>>> - it looks like there separate delimiters defined in 2 application parts: >>>>> a) mailbox separator (defined in MailboxConstants), >>>>> b) imap hierarchy delimiter (defined in ImapConstants) >>>>> >>>>> they interact with each other - so I guess they should be unified >>>>> to be the same after refactoring - am I right? >>> >>>> Exactly. I think the delimiter should be configured in the >>>> MailboxManager or MailboxSession (?) and then just looked-up in the >>>> imap processors. >>> >>> >>>>> >>>>> - delimiter is represented as 'String' in some places and as 'char' >>>>> in the others - should we unify this? Will be char enough? Are there >>>>> cases where it will be NULL (anyway should be probably represented as >>>>> chr(0)) or multi character string? >>> >>>> Yeah make this consistent is a good idea.. Char should be enough to >>>> hold the delimiter. The delimiter should never be NULL. >>> >>>>> >>>>> >>>>> >>>>> >>>>>> There are no stupid questions.... Just keep asking I will try to guide >>>>>> you as much As possible. >>>>> >>>>>> I think you will have to add the delimiter stuff to MailboxSession and >>>>>> replace the hardcoded Value with the One in MailboxSession >>>>> >>>>>> Good luck;) >>>>> >>>>>> Am Donnerstag, 30. Dezember 2010 schrieb Wojciech Strzałka >>>>>> <[email protected]>: >>>>>>> >>>>>>> I don't feel comfortable yet, with the overall project structure yet >>>>>>> but >>>>>>> I can try :D >>>>>>> Expect stupid questions on priv :) >>>>>>> >>>>>>>> Good point, >>>>>>> >>>>>>>> I'm not sure I have time before 0.2 to implement this, but if you want >>>>>>>> and have time you could write a patch. We love contributions and I >>>>>>>> would be more then happy to review your work ;) >>>>>>> >>>>>>>> Bye, >>>>>>>> Norman >>>>>>> >>>>>>> >>>>>>>> 2010/12/30 Wojciech Strzałka <[email protected]>: >>>>>>>>> >>>>>>>>> ... a system where path separator is "/" and "." is allowed >>>>>>>>> character in folder name. >>>>>>>>> >>>>>>>>>> Hmm, >>>>>>>>> >>>>>>>>>> could you give me a use case for this ? Just try to understand why >>>>>>>>>> you >>>>>>>>>> want todo this.. >>>>>>>>> >>>>>>>>>> Bye, >>>>>>>>>> Norman >>>>>>>>> >>>>>>>>>> 2010/12/30 Wojciech Strzałka <[email protected]>: >>>>>>>>>>> >>>>>>>>>>> One of the missing pieces is possibility to change default server >>>>>>>>>>> hierarchy separator. Currently "." is fixed value. >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> --------------------------------------------------------------------- >>>>>>>>>>> To unsubscribe, e-mail: [email protected] >>>>>>>>>>> For additional commands, e-mail: [email protected] >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>> >>>>>>>>>> --------------------------------------------------------------------- >>>>>>>>>> To unsubscribe, e-mail: [email protected] >>>>>>>>>> For additional commands, e-mail: [email protected] >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> -- >>>>>>>>> Pozdrowienia, >>>>>>>>> Wojciech Strzałka >>>>>>>>> >>>>>>>>> >>>>>>>>> --------------------------------------------------------------------- >>>>>>>>> To unsubscribe, e-mail: [email protected] >>>>>>>>> For additional commands, e-mail: [email protected] >>>>>>>>> >>>>>>>>> >>>>>>> >>>>>>>> --------------------------------------------------------------------- >>>>>>>> To unsubscribe, e-mail: [email protected] >>>>>>>> For additional commands, e-mail: [email protected] >>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> Pozdrowienia, >>>>>>> Wojciech Strzałka >>>>>>> >>>>>>> >>>>>>> --------------------------------------------------------------------- >>>>>>> To unsubscribe, e-mail: [email protected] >>>>>>> For additional commands, e-mail: [email protected] >>>>>>> >>>>>>> >>>>> >>>>>> --------------------------------------------------------------------- >>>>>> To unsubscribe, e-mail: [email protected] >>>>>> For additional commands, e-mail: [email protected] >>>>> >>>>> >>>>> >>>>> -- >>>>> Pozdrowienia, >>>>> Wojciech Strzałka >>>>> >>> >>>> Bye, >>>> Norman >>> >>>> Ps: I CC server-dev as this is the right place for the mail thread.. >>> >>> >>> >>> -- >>> Pozdrowienia, >>> Wojciech Strzałka > > > > -- > Pozdrowienia, > Wojciech Strzałka > > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
