Arsnael commented on code in PR #2410:
URL: https://github.com/apache/james-project/pull/2410#discussion_r1760590779
##########
protocols/imap/src/main/java/org/apache/james/imap/main/DefaultImapDecoderFactory.java:
##########
@@ -27,20 +27,35 @@
import org.apache.james.imap.message.response.UnpooledStatusResponseFactory;
/**
- * TODO: this is temporary: should let the container do the coupling. TODO:
- * convert to POJO
+ * Factory class for creating `ImapDecoder` instances.
+ *
+ * This class is a POJO that manually manages its dependencies.
+ * Dependencies are injected through the constructor, which allows for
+ * better decoupling and easier testing.
+ *
+ * The creation of `ImapCommandParserFactory` is handled internally by
+ * this factory, based on the provided `UnpooledStatusResponseFactory`.
*/
public class DefaultImapDecoderFactory implements ImapDecoderFactory {
- public static ImapDecoder createDecoder() {
- final UnpooledStatusResponseFactory unpooledStatusResponseFactory =
new UnpooledStatusResponseFactory();
- final ImapCommandParserFactory imapCommands = new
ImapParserFactory(unpooledStatusResponseFactory);
- return new DefaultImapDecoder(unpooledStatusResponseFactory,
imapCommands);
+ private final UnpooledStatusResponseFactory unpooledStatusResponseFactory;
+ private final ImapCommandParserFactory imapCommandParserFactory;
+
+ /**
+ * Constructs `DefaultImapDecoderFactory` with the given
+ * `UnpooledStatusResponseFactory`. The `ImapCommandParserFactory`
+ * is created internally using the provided
`UnpooledStatusResponseFactory`.
+ *
+ * @param unpooledStatusResponseFactory The factory for creating status
responses.
+ */
+ public DefaultImapDecoderFactory(UnpooledStatusResponseFactory
unpooledStatusResponseFactory) {
Review Comment:
That code is very old... Aint very sure about this part but problems I can
see:
- several test classes depend on
DefaultImapDecoderFactory.buildImapDecoder() . This change without being
adapted is definitely gonna break them
- have a look in `IMAPServerModule` . The DefaultImapDecoder is being
provided by building directly it, not passing by the Factory. It would make
sense maybe to do a refactoring there as well.
- you might want to pass the ImapCommandParserFactory as an argument too (to
use in IMAPServerModule)
- you could maybe have a static method that would return a default imap
encoder (as of now) just for tests purposes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org
