Thanks to those who have replied so far. I am going to try to ask better questions. What I am trying to figure out is how James models the SMTP concepts in the code. I am very particular in that I think it is extremely important to make the code as “readable” as possible. I wanted to hear the thoughts behind the current model.
I will dive in and tell you what I see, and ask some more questions. :-)
Please remember that I represent those who are new to James and who want to use
it and understand it. I also want to be able to document it better, which means
that I need to understand it better.
I started my journey with investigating the SMTP protocol api and
implementation in James. I am specifically looking at the protocols-smtp
project. Here is the train of thoughts I had as I tried to navigate the code to
figure out how James models smtp.
The first place I naturally look is the package
org.apache.james.protocols.smtp. The package seems to be well-named: it is
appropriate and easy to understand.
Within this package, there are sub-packages: core, dsn, and hook. Those are
definitely not the first place I would look to understand the smtp api. Within
the interfaces / classes in the “main” package (i.e.
“org.apache.james.protocols.smtp”), I find:
AllButStartTlsLineBasedChannelHandler.java
AllButStartTlsLineChannelHandlerFactory.java
CommandInjectionDetectedException.java
MailAddress.java
MailAddressException.java
MailEnvelope.java
MailEnvelopeImpl.java
SMTPConfiguration.java
SMTPConfigurationImpl.java
SMTPProtocol.java
SMTPProtocolHandlerChain.java
SMTPResponse.java
SMTPRetCode.java
SMTPServerMBean.java
SMTPSession.java
SMTPSessionImpl.java
SMTPStartTlsResponse.java
Within these, MailAddress and MailAddressException are deprecated, so I ignore
them.
My first reactions are:
* AllButStartTlsLineBasedChannelHandler* … wtf??? Ok, don’t want to get into
that right now.
* Why are there impl classes in the main api package????
* Why is there an MBean class polluting the api???
* SMTPProtocol looks like the most sensical place to start, so…
I look at SMTPProtocol. My thoughts are:
* Holy crap, this is a class, not an interface. What’s going on here??
* Ok, it implements org.apache.james.protocols.api.ProtocolImpl, which itself
implements org.apache.james.protocols.api.Protocol
- Already this is confusing and there is a lot of coupling going on here.
Arg.
Ok, first let’s look at the Protocol API.
Ok, this is an interface, so I am indeed in api territory. I feel a bit better.
But why is the protocols-smtp api coupled to the protocols-api project? That is
not clear. Is there really a benefit to this inheritance? Coupling apis is
generally quite yucky. Is the idea that ALL protocols are the same?
My thought is that even if there is overlap between the protocol apis, it is a
bit dubious to create this kind of coupling just because it may save a few
lines of code. It has a fairly high risk of creating a world of pain in terms
of causing unnecessary coupling and more difficult dependency management later
on.
—> I understand that there are “protocols” and that they each “do something”,
but can somebody explain the decision to use this kind of inheritance to try to
define all of the protocols as a single “thing", instead of having independent
apis per protocol? Is there some benefit to this coupling that I am not seeing?
Anyway, to continue…
A Protocol has a ProtocolHandlerChain, a ProtocolConfiguration, and a
ProtocolSession. Alrighty, let’s look at those.
ProtocolHandlerChain - this is in the org.apache.james.protocols.api.handler
package, which means that the “main” package depends on a sub-package. That
seems backwards to me. Anyway, when I look at this interface, I kinda get a
sense of what it does, but it is soooo abstract and there is so little
documentation that really I can only guess as to what it is for.
ProtocolConfiguration - this is also in the “main” package, which is fine. This
is very abstract, but since it represents a “Configuration” I think that’s ok.
However, the provided documentation is pretty much useless. It only repeats the
name of the interface methods without giving any additional information as to
why these methods exist in the first place.
ProtocolSession - This opens up a whole new pandora’s box that I am not ready
to investigate right now. I’ll have to try to bite this part of the elephant
later.
Ok, so back to… ummmm, where was I? I’m already lost, so I’ll have to start
again at SMTPProtocol and try to find my way again.
Well, maybe I’ve already said enough for now anyway. I’ll leave it here for
now, await responses, and continue again later.
My incomplete (and maybe unjustified??) conclusions so far:
* There is a lot of coupling that I don’t understand and that makes the api
already more complex than I would like
* There are many very abstract concepts that make comprehension very difficult
for me
* The api and implementation classes are all mixed together, which I find
utterly confusing
Not sure if this is helpful or not, but I think that the ability to comprehend
the codebase is very important, especially in a large and complex system in
which many people collaborate.
Hopefully people can answer some of the questions I pose in my thoughts above.
Cheers,
=David
signature.asc
Description: Message signed with OpenPGP
