Never mind. My bad. The dependency was because I've used ImapSessionUtils.getMailboxSession() - while it's one liner I will embed it into CreateCommandParser::decode()
> 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 >> >> -- Pozdrowienia, Wojciech Strzałka --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
