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]
