You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by Norman Maurer <nm...@byteaction.de> on 2006/07/11 17:17:55 UTC

smtp-handler-api and merge to trunk

Hi guys,

after workin a while on the smtp-handler-api now i think it is  ready to
get merged to trunk.

Here are the things i change.

1. I add a new method to the CommandHandler interface called
getImplCommands(). This method return the commands (as ArrayList) which
can use the CommandHandler. So we can be sure that noone use a "wrong"
CommandHandler. If a "wrong" commandHandler is configured for the
command an ConfigurationException is thrown on startup.

2. I seperate all the "filter" stuff (syntaxchecks etc) from the
commandhandler and move it to an extra package. 

3. I add a interface called CommandsHandler. This interface has a method
called getCommands() which return a Map that contain commands and the
comanndhandlers which should be called for the command.


4. I add a class called BaseCmdHandler which implement CommandsHandler.
This class load all "core" commandHandlers. 

5. I add a class called BaseFilterCmdHandlr which implement
CommandsHandler. This class load all "core" filters for the
commandHandlers. 

6. Create a package called fastfailfilters which now contain all the
fastfail code we had before in the corehandlers.

7. Make it possible to configure a commandHandler for more then on
command with: <class="org.apache.james.commandHandler.foobar"
command="HELO,EHLO">

8. Create a 2 new methods in SMTPSession to set whenether the next
commandHandler should be executed or not.

9. Change the SMTPHandlerChain to load at least the core commands.


I hope i explain all core changes i made..
So what you guys think about mergin it in the trunk ?

bye
                                 Norman

Re: smtp-handler-api and merge to trunk

Posted by Stefano Bagnara <ap...@bago.org>.
Noel J. Bergman wrote:
>> We have the same problems that we have in Mailets: if you need a service
>> you have to be Serviceable or to lookup the ServiceManager from the
> Context.
> 
> Mailets ought to access key services via the MailetContext.  That is why we
> have MailetConfig and MailetContext, just like the Servlet API.  Mailets
> were never supposed to deal with Avalon.  The few that had to were
> aberrations that were supposed to be fixed by improving the Mailet
> environment.

I'm not sure I agree with you on this.
James is a container itself and should provide an easy way for people 
that want to add new services/components and use this services in the 
mailets. The same system should be used by James itself.
I consider the current way to access the ServiceManager via a property 
in the context an HACK and we have A LOT of mailets doing this, not FEW.


>> Imho moving to init is probably a bad move: I prefer to have a clear
>> dependency on Avalon instead of having an hidden/subtle dependency on
>> Avalon.
> 
> I want NO dependency on Avalon.  The handlers should follow the same
> pattern.  The key services can be exposed by passing in a config from which
> we can get to a HandlerContext.
> 
> 	--- Noel

I'm -1 about putting all the services that we have in James inside the 
SMTPHandlerSession or put all of the in the context. This would be 
really a bad thing to our modularity.

Imho it is really better to depends on Avalon for correct encapsulation 
and IoC than spreading services all around because we don't have a 
container (or don't want to depend on one).

Maybe I didn't understand your solution, but I think that having the 
ServiceManager in the context is not a better solution than adding 
Avalon interfaces to that objects! You have the avalon dependency either 
way, but if you do that via context you "hide it" and make it more 
difficult to manage.

Stefano


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


RE: smtp-handler-api and merge to trunk

Posted by "Noel J. Bergman" <no...@devtech.com>.
Stefano Bagnara wrote:

> Noel J. Bergman wrote:
> > For example, by the time we're
> > done, I expect that configure() will be replaced by init(), and
> > we'll have eliminated the Avalon API from polluting the code.

> I don't think that eliminating Avalon API from the code will be so easy.

It won't be so hard if we stop propogating the Avalon API where it doesn't
belong and isn't needed.

> We have the same problems that we have in Mailets: if you need a service
> you have to be Serviceable or to lookup the ServiceManager from the
Context.

Mailets ought to access key services via the MailetContext.  That is why we
have MailetConfig and MailetContext, just like the Servlet API.  Mailets
were never supposed to deal with Avalon.  The few that had to were
aberrations that were supposed to be fixed by improving the Mailet
environment.

> Imho moving to init is probably a bad move: I prefer to have a clear
> dependency on Avalon instead of having an hidden/subtle dependency on
> Avalon.

I want NO dependency on Avalon.  The handlers should follow the same
pattern.  The key services can be exposed by passing in a config from which
we can get to a HandlerContext.

	--- Noel


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


Re: smtp-handler-api and merge to trunk

Posted by Stefano Bagnara <ap...@bago.org>.
Noel J. Bergman wrote:
> For example, by the time we're
> done, I expect that configure() will be replaced by init(), and we'll have
> eliminated the Avalon API from polluting the code.

I don't think that eliminating Avalon API from the code will be so easy.

We have the same problems that we have in Mailets: if you need a service 
you have to be Serviceable or to lookup the ServiceManager from the Context.

Imho moving to init is probably a bad move: I prefer to have a clear 
dependency on Avalon instead of having an hidden/subtle dependency on 
Avalon.

Btw I don't have time for religious container wars ;-) .. I prefer to 
keep it to work on my handlerapi proposal and maybe you will like the 
"plugin mechanism" I have in mind and maybe we can use it something 
similar for Mailets.

As an hint my proposal will include a generic "extension point 
mechanism" using an extension manager and "enabling interfaces" for the 
dependency injection.

Btw maybe I will not find the time to work on it, and there won't be any 
need to discuss about my proposal at all ;-)

Furthermore, generically speaking, I love modularity but I think that 
modularity is a weapong to be used with much wisdom. We should never 
introduce a modularity that is not needed in the real life because we 
probably only add ways to break the system and nothing more.

Stefano


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


Re: smtp-handler-api and merge to trunk

Posted by Stefano Bagnara <ap...@bago.org>.
Noel J. Bergman wrote:
> Stefano Bagnara wrote:
> 
>> I have already worked locally on a much different architecture for the
>> smtphandler stuff
> 
>> my proposal has started from the current smtphandler branch, so I
>> think I could use the current handlerapi branch to show you my proposal
> 
> Can you highlight the differences?  You say architecture.  Is it an
> architecture difference, or API tweaking?  For example, by the time we're
> done, I expect that configure() will be replaced by init(), and we'll have
> eliminated the Avalon API from polluting the code.

You will see some code as soon as I will have more time to work on this.
If in the mean time the current handlerapi branch had been merged I will 
directly use the handlerapi branch to show you my ideas, otherwise I'll 
open a JIRA issue and I'll attach code there.

Stefano


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


RE: smtp-handler-api and merge to trunk

Posted by "Noel J. Bergman" <no...@devtech.com>.
Stefano Bagnara wrote:

> I have already worked locally on a much different architecture for the
> smtphandler stuff

> my proposal has started from the current smtphandler branch, so I
> think I could use the current handlerapi branch to show you my proposal

Can you highlight the differences?  You say architecture.  Is it an
architecture difference, or API tweaking?  For example, by the time we're
done, I expect that configure() will be replaced by init(), and we'll have
eliminated the Avalon API from polluting the code.

	--- Noel


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


Re: smtp-handler-api and merge to trunk

Posted by Stefano Bagnara <ap...@bago.org>.
Norman Maurer wrote:
> after workin a while on the smtp-handler-api now i think it is  ready to
> get merged to trunk.
> 
> Here are the things i change.
> [...]
> I hope i explain all core changes i made..
> So what you guys think about mergin it in the trunk ?

I'm +0 on the proposal itself, but I'm +1 about this commit to trunk.

Let me explain.

I have already worked locally on a much different architecture for the 
smtphandler stuff but I had no time (2.3 blocking issues + day job 
issues + bicycle accident) to really work on it.

Btw my proposal has started from the current smtphandler branch, so I 
think I could use the current handlerapi branch to show you my proposal 
once the current code is in trunk.

+0 stands for: I don't think this a good "final" solution for our 
pluggable handler api but I think this could be a first step forward.

Stefano


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


RE: smtp-handler-api and merge to trunk

Posted by "Noel J. Bergman" <no...@devtech.com>.
Norman Maurer wrote:

> 1. I add a new method to the CommandHandler interface called
>    getImplCommands(). This method return the commands
>   (as ArrayList)

Why not a Collection or a Map?

> If a "wrong" commandHandler is configured for the command an
> ConfigurationException is thrown

Do you mean the wrong command for a Handler?  For example,

  <handler class="myRCPThandler" command="HELO"/>

> 2. I seperate all the "filter" stuff (syntaxchecks etc) from the
> commandhandler and move it to an extra package.

We could also move all of the core handlers into the core package.  And we
should be clear that filters ARE handlers.  You've just chosen a name for
semantic purposes.

> 8. Create a 2 new methods in SMTPSession to set whenether the next
>    commandHandler should be executed or not.

Not sure if that is the right way to do it, but we can see.

+1 to merge to trunk as a strawman.  Curious to see what differences Stefano
has in mind.

	--- Noel


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