[jsieve] Refactoring

[Robert Burrell Donkin <ro...@gmail.com>]

i think that it should be possible to remove the singletons by two
independent refactorings:

 * Separating configuration concerns using the factory pattern
 * Separating state concerns using a context

opinions welcomed

unless anyone jumps in soon, i'll take a shot at committing a first
draft sometime soon. a postumous review phase will probably be needed.

- robert

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org

Re: [jsieve] Refactoring

[Robert Burrell Donkin <ro...@gmail.com>]

On Tue, Sep 2, 2008 at 6:00 PM, Robert Burrell Donkin
<ro...@gmail.com> wrote:
> On Tue, Sep 2, 2008 at 11:52 AM, Stefano Bagnara <ap...@bago.org> wrote:

<snip>

>> 1) code reformat: use JAMES conventions for this ({ should not be at the
>> beginning of lines, mainly). reformatting make it more difficult to track
>> changes, so it is better to do that when big refactorings happen (as the
>> first or the last change).
>
> i'm agnostic but i agree that using a single convention usually makes
> this easier
>
> might as well do this now

done

- robert

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org

Re: [jsieve] Refactoring

[Robert Burrell Donkin <ro...@gmail.com>]

On Tue, Sep 2, 2008 at 11:52 AM, Stefano Bagnara <ap...@bago.org> wrote:
> Robert Burrell Donkin ha scritto:
>>
>> On Sun, Aug 31, 2008 at 2:24 PM, Stefano Bagnara <ap...@bago.org> wrote:
>>
>> <snip>
>>
>>> ATM it is almost impossible to make a dependency graph for jsieve because
>>> components are too much interconnected and there are too many cycles
>>> between
>>> packages and classes.
>>>
>>> E.g: IMHO there is no clean separation between parsing and running
>>> scripts.
>>> I hope to be able to do some reorganization of the code/packages once you
>>> will have refactored it.
>>
>> take another look now: it's not done but now's probably a good time to
>> work out how to complete the work
>
> I updated it (and cleaned junit tests), I'll look better at it later.

ok

i may take a look at moving some code around to improve entry and usage

> In the mean time I write some things that IMHO could be done there:
>
> 1) code reformat: use JAMES conventions for this ({ should not be at the
> beginning of lines, mainly). reformatting make it more difficult to track
> changes, so it is better to do that when big refactorings happen (as the
> first or the last change).

i'm agnostic but i agree that using a single convention usually makes
this easier

might as well do this now

> 2) "samples" are currently in the folder but not used by mvn or ant build.
> They depends on james libraries (mainly the javamail-util module) and on
> mailet-api library. Maybe jsieve should be a multimodule too and samples
> should be the second module.. any other option?

not sure

multi-module would allow a couple of additional micro-libraries with
different dependencies to be included

> 3) I don't like ".junit." in the test package. I'm used to tests in the same
> package of the class under test.

+1

- robert

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org

Re: [jsieve] Refactoring

[Stefano Bagnara <ap...@bago.org>]

Robert Burrell Donkin ha scritto:
> On Sun, Aug 31, 2008 at 2:24 PM, Stefano Bagnara <ap...@bago.org> wrote:
> 
> <snip>
> 
>> ATM it is almost impossible to make a dependency graph for jsieve because
>> components are too much interconnected and there are too many cycles between
>> packages and classes.
>>
>> E.g: IMHO there is no clean separation between parsing and running scripts.
>> I hope to be able to do some reorganization of the code/packages once you
>> will have refactored it.
> 
> take another look now: it's not done but now's probably a good time to
> work out how to complete the work

I updated it (and cleaned junit tests), I'll look better at it later.

In the mean time I write some things that IMHO could be done there:

1) code reformat: use JAMES conventions for this ({ should not be at the 
beginning of lines, mainly). reformatting make it more difficult to 
track changes, so it is better to do that when big refactorings happen 
(as the first or the last change).

2) "samples" are currently in the folder but not used by mvn or ant 
build. They depends on james libraries (mainly the javamail-util module) 
and on mailet-api library. Maybe jsieve should be a multimodule too and 
samples should be the second module.. any other option?

3) I don't like ".junit." in the test package. I'm used to tests in the 
same package of the class under test.

WDYT?

Stefano

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org

Re: [jsieve] Refactoring

[Robert Burrell Donkin <ro...@gmail.com>]

On Sun, Aug 31, 2008 at 2:24 PM, Stefano Bagnara <ap...@bago.org> wrote:

<snip>

> ATM it is almost impossible to make a dependency graph for jsieve because
> components are too much interconnected and there are too many cycles between
> packages and classes.
>
> E.g: IMHO there is no clean separation between parsing and running scripts.
> I hope to be able to do some reorganization of the code/packages once you
> will have refactored it.

take another look now: it's not done but now's probably a good time to
work out how to complete the work

- robert

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org

Re: [jsieve] Refactoring

[Robert Burrell Donkin <ro...@gmail.com>]

On Sun, Aug 31, 2008 at 2:24 PM, Stefano Bagnara <ap...@bago.org> wrote:
> Robert Burrell Donkin ha scritto:
>>
>> i think that it should be possible to remove the singletons by two
>> independent refactorings:
>>
>>  * Separating configuration concerns using the factory pattern
>
> Do you mean having a factory to generate all of the services needed by a
> script parsing process and then have services injected or injecting service
> factories to each component?

IMHO JSieve is best understood as a single component with no service
dependencies

AIUI ATM there's really only way to configure JSieve. the information
required is loaded from 3 property files containing the classnames of
tests, comparators and command bindings. this configuration is pure
assembly, no services are required.

>>  * Separating state concerns using a context
>
> We already use "Context" for SieveContext that is a script level context.
> Maybe "Session" is a better term for a script execution context.

context is the term i've seen commonly used for this pattern (for
example, in xml binding) but i agree that session would convey the
appropriate meaning. SieveContext is really a session.

> Maybe both ConditionManager and CommandStateManager "states" can be moved to
> a simple SieveSession (or simply Session) object and the SieveSession could
> even expose generic set/get properties to allow simpler extensibility (new
> commands would not depend on improvements in the core session object).

(this pattern is one i favour so i'm may probably biased but) yes

>> opinions welcomed
>>
>> unless anyone jumps in soon, i'll take a shot at committing a first
>> draft sometime soon. a postumous review phase will probably be needed.
>
> ATM it is almost impossible to make a dependency graph for jsieve because
> components are too much interconnected and there are too many cycles between
> packages and classes.
>
> E.g: IMHO there is no clean separation between parsing and running scripts.
> I hope to be able to do some reorganization of the code/packages once you
> will have refactored it.

+1

once the singletons are gone, there's probably more work needed
factoring into independent parser and execution units. ATM it is
necessary to repeatedly parse scripts even when the documents haven't
changed. probably need to think about separating concerns. probably
need something more like:

configurator --creates--> factory --creates--> parser --creates --> executor

- robert

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org

Re: [jsieve] Refactoring

[Stefano Bagnara <ap...@bago.org>]

Robert Burrell Donkin ha scritto:
> i think that it should be possible to remove the singletons by two
> independent refactorings:
> 
>  * Separating configuration concerns using the factory pattern

Do you mean having a factory to generate all of the services needed by a 
script parsing process and then have services injected or injecting 
service factories to each component?

>  * Separating state concerns using a context

We already use "Context" for SieveContext that is a script level 
context. Maybe "Session" is a better term for a script execution context.

Maybe both ConditionManager and CommandStateManager "states" can be 
moved to a simple SieveSession (or simply Session) object and the 
SieveSession could even expose generic set/get properties to allow 
simpler extensibility (new commands would not depend on improvements in 
the core session object).

> opinions welcomed
> 
> unless anyone jumps in soon, i'll take a shot at committing a first
> draft sometime soon. a postumous review phase will probably be needed.

ATM it is almost impossible to make a dependency graph for jsieve 
because components are too much interconnected and there are too many 
cycles between packages and classes.

E.g: IMHO there is no clean separation between parsing and running 
scripts. I hope to be able to do some reorganization of the 
code/packages once you will have refactored it.

Stefano

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org