Ah ok.. Anyway I think IMapSessionUtils should get moved to api... wdyt ?
Bye, Norman 2011/1/7 Wojciech Strzałka <[email protected]>: > > 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]
