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 pg...@apache.org on 2002/10/26 06:15:31 UTC

cvs commit: jakarta-james/src/java/org/apache/james/util/watchdog BytesReadResetInputStream.java BytesWrittenResetOutputStream.java InaccurateTimeoutWatchdog.java SchedulerWatchdogFactory.java ThreadPerWatchdogFactory.java Watchdog.java WatchdogFactory.java WatchdogTarget.java package.html

pgoldstein    2002/10/25 21:15:31

  Modified:    src/java/org/apache/james/nntpserver NNTPHandler.java
                        NNTPServer.java NNTPServer.xinfo
               src/java/org/apache/james/nntpserver/repository
                        NNTPArticleImpl.java NNTPGroup.java
                        NNTPGroupImpl.java NNTPRepository.java
                        NNTPRepositoryImpl.java NNTPSpooler.java
               src/conf james-assembly.xml james-server.xml
               src/java/org/apache/james/pop3server POP3Handler.java
                        POP3Server.java POP3Server.xinfo
               src/java/org/apache/james/remotemanager RemoteManager.java
                        RemoteManager.xinfo RemoteManagerHandler.java
               src/java/org/apache/james/core MailImpl.java
                        MimeMessageInputStreamSource.java
                        MimeMessageSource.java MimeMessageWrapper.java
               src/java/org/apache/james/smtpserver SMTPHandler.java
                        SMTPServer.java SMTPServer.xinfo
  Added:       src/java/org/apache/james/nntpserver
                        NNTPHandlerConfigurationData.java
               src/java/org/apache/james/pop3server
                        POP3HandlerConfigurationData.java
               src/java/org/apache/james/remotemanager
                        RemoteManagerHandlerConfigurationData.java
               src/java/org/apache/james/core AbstractJamesService.java
               src/java/org/apache/james/smtpserver
                        SMTPHandlerConfigurationData.java
               src/java/org/apache/james/util/watchdog
                        BytesReadResetInputStream.java
                        BytesWrittenResetOutputStream.java
                        InaccurateTimeoutWatchdog.java
                        SchedulerWatchdogFactory.java
                        ThreadPerWatchdogFactory.java Watchdog.java
                        WatchdogFactory.java WatchdogTarget.java
                        package.html
  Removed:     src/java/org/apache/james BaseConnectionHandler.java
               src/java/org/apache/james/nntpserver ArticleWriter.java
  Log:
  A variety of changes.
  Corrected a memory leak
  Pooled handlers
  Introduced AbstractJamesService class as the base service class for James services
  
  Revision  Changes    Path
  1.19      +627 -211  jakarta-james/src/java/org/apache/james/nntpserver/NNTPHandler.java
  
  Index: NNTPHandler.java
  ===================================================================
  RCS file: /home/cvs/jakarta-james/src/java/org/apache/james/nntpserver/NNTPHandler.java,v
  retrieving revision 1.18
  retrieving revision 1.19
  diff -u -r1.18 -r1.19
  --- NNTPHandler.java	4 Oct 2002 08:17:49 -0000	1.18
  +++ NNTPHandler.java	26 Oct 2002 04:15:29 -0000	1.19
  @@ -8,30 +8,34 @@
   package org.apache.james.nntpserver;
   
   import org.apache.avalon.cornerstone.services.connection.ConnectionHandler;
  -import org.apache.avalon.cornerstone.services.scheduler.PeriodicTimeTrigger;
  -import org.apache.avalon.cornerstone.services.scheduler.Target;
  -import org.apache.avalon.cornerstone.services.scheduler.TimeScheduler;
  +import org.apache.avalon.excalibur.pool.Poolable;
  +import org.apache.avalon.framework.activity.Disposable;
   import org.apache.avalon.framework.component.ComponentException;
   import org.apache.avalon.framework.component.ComponentManager;
   import org.apache.avalon.framework.component.Composable;
   import org.apache.avalon.framework.configuration.Configurable;
   import org.apache.avalon.framework.configuration.Configuration;
   import org.apache.avalon.framework.configuration.ConfigurationException;
  +import org.apache.avalon.framework.logger.AbstractLogEnabled;
   import org.apache.avalon.framework.logger.Logger;
  -import org.apache.james.BaseConnectionHandler;
   import org.apache.james.nntpserver.repository.NNTPArticle;
   import org.apache.james.nntpserver.repository.NNTPGroup;
   import org.apache.james.nntpserver.repository.NNTPLineReaderImpl;
   import org.apache.james.nntpserver.repository.NNTPRepository;
   import org.apache.james.services.UsersRepository;
   import org.apache.james.services.UsersStore;
  +import org.apache.james.util.InternetPrintWriter;
   import org.apache.james.util.RFC977DateFormat;
   import org.apache.james.util.RFC2980DateFormat;
   import org.apache.james.util.SimplifiedDateFormat;
  +import org.apache.james.util.watchdog.Watchdog;
  +import org.apache.james.util.watchdog.WatchdogTarget;
   
   import java.io.BufferedReader;
  +import java.io.BufferedWriter;
   import java.io.IOException;
   import java.io.InputStreamReader;
  +import java.io.OutputStreamWriter;
   import java.io.PrintWriter;
   import java.net.Socket;
   import java.text.DateFormat;
  @@ -49,8 +53,9 @@
    * @author Harmeet <hb...@apache.org>
    * @author Peter M. Goldstein <fa...@alum.mit.edu>
    */
  -public class NNTPHandler extends BaseConnectionHandler
  -    implements ConnectionHandler, Composable, Configurable, Target {
  +public class NNTPHandler
  +    extends AbstractLogEnabled
  +    implements ConnectionHandler, Poolable {
   
       /**
        * used to calculate DATE from - see 11.3
  @@ -68,9 +73,134 @@
       public static final long UTC_OFFSET = Calendar.getInstance().get(Calendar.ZONE_OFFSET);
   
       /**
  -     * Timeout controller
  +     * The text string for the NNTP MODE command.
        */
  -    private TimeScheduler scheduler;
  +    private final static String COMMAND_MODE = "MODE";
  +
  +    /**
  +     * The text string for the NNTP LIST command.
  +     */
  +    private final static String COMMAND_LIST = "LIST";
  +
  +    /**
  +     * The text string for the NNTP GROUP command.
  +     */
  +    private final static String COMMAND_GROUP = "GROUP";
  +
  +    /**
  +     * The text string for the NNTP NEXT command.
  +     */
  +    private final static String COMMAND_NEXT = "NEXT";
  +
  +    /**
  +     * The text string for the NNTP LAST command.
  +     */
  +    private final static String COMMAND_LAST = "LAST";
  +
  +    /**
  +     * The text string for the NNTP ARTICLE command.
  +     */
  +    private final static String COMMAND_ARTICLE = "ARTICLE";
  +
  +    /**
  +     * The text string for the NNTP HEAD command.
  +     */
  +    private final static String COMMAND_HEAD = "HEAD";
  +
  +    /**
  +     * The text string for the NNTP BODY command.
  +     */
  +    private final static String COMMAND_BODY = "BODY";
  +
  +    /**
  +     * The text string for the NNTP STAT command.
  +     */
  +    private final static String COMMAND_STAT = "STAT";
  +
  +    /**
  +     * The text string for the NNTP POST command.
  +     */
  +    private final static String COMMAND_POST = "POST";
  +
  +    /**
  +     * The text string for the NNTP IHAVE command.
  +     */
  +    private final static String COMMAND_IHAVE = "IHAVE";
  +
  +    /**
  +     * The text string for the NNTP QUIT command.
  +     */
  +    private final static String COMMAND_QUIT = "QUIT";
  +
  +    /**
  +     * The text string for the NNTP DATE command.
  +     */
  +    private final static String COMMAND_DATE = "DATE";
  +
  +    /**
  +     * The text string for the NNTP HELP command.
  +     */
  +    private final static String COMMAND_HELP = "HELP";
  +
  +    /**
  +     * The text string for the NNTP NEWGROUPS command.
  +     */
  +    private final static String COMMAND_NEWGROUPS = "NEWGROUPS";
  +
  +    /**
  +     * The text string for the NNTP NEWNEWS command.
  +     */
  +    private final static String COMMAND_NEWNEWS = "NEWNEWS";
  +
  +    /**
  +     * The text string for the NNTP LISTGROUP command.
  +     */
  +    private final static String COMMAND_LISTGROUP = "LISTGROUP";
  +
  +    /**
  +     * The text string for the NNTP OVER command.
  +     */
  +    private final static String COMMAND_OVER = "OVER";
  +
  +    /**
  +     * The text string for the NNTP XOVER command.
  +     */
  +    private final static String COMMAND_XOVER = "XOVER";
  +
  +    /**
  +     * The text string for the NNTP HDR command.
  +     */
  +    private final static String COMMAND_HDR = "HDR";
  +
  +    /**
  +     * The text string for the NNTP XHDR command.
  +     */
  +    private final static String COMMAND_XHDR = "XHDR";
  +
  +    /**
  +     * The text string for the NNTP AUTHINFO command.
  +     */
  +    private final static String COMMAND_AUTHINFO = "AUTHINFO";
  +
  +    /**
  +     * The text string for the NNTP MODE READER parameter.
  +     */
  +    private final static String MODE_TYPE_READER = "READER";
  +
  +    /**
  +     * The text string for the NNTP MODE STREAM parameter.
  +     */
  +    private final static String MODE_TYPE_STREAM = "STREAM";
  +
  +    /**
  +     * The text string for the NNTP AUTHINFO USER parameter.
  +     */
  +    private final static String AUTHINFO_PARAM_USER = "USER";
  +
  +    /**
  +     * The text string for the NNTP AUTHINFO PASS parameter.
  +     */
  +    private final static String AUTHINFO_PARAM_PASS = "PASS";
   
       /**
        * The TCP/IP socket over which the POP3 interaction
  @@ -94,19 +224,15 @@
       private NNTPGroup group;
   
       /**
  -     * The repository that stores the news articles for this NNTP server.
  -     */
  -    private NNTPRepository repo;
  -
  -    /**
  -     * The repository that stores the local users.  Used for authentication.
  +     * The current newsgroup.
        */
  -    private UsersRepository userRepository = null;
  +    private int currentArticleNumber = -1;
   
       /**
  -     * Whether authentication is required to access this NNTP server
  +     * Per-service configuration data that applies to all handlers
  +     * associated with the service.
        */
  -    private boolean authRequired = false;
  +    private NNTPHandlerConfigurationData theConfigData;
   
       /**
        * The user id associated with the NNTP dialogue
  @@ -118,133 +244,155 @@
        */
       private String password;
   
  +    /**
  +     * The watchdog being used by this handler to deal with idle timeouts.
  +     */
  +    private Watchdog theWatchdog;
   
       /**
  -     * @see org.apache.avalon.framework.component.Composable#compose(ComponentManager)
  +     * The watchdog target that idles out this handler.
        */
  -    public void compose( final ComponentManager componentManager )
  -        throws ComponentException
  -    {
  -        getLogger().debug("NNTPHandler compose...begin");
  -        UsersStore usersStore = (UsersStore)componentManager.lookup(UsersStore.ROLE);
  -        userRepository = usersStore.getRepository("LocalUsers");
  +    private WatchdogTarget theWatchdogTarget = new NNTPWatchdogTarget();
   
  -        scheduler = (TimeScheduler)componentManager.
  -            lookup( "org.apache.avalon.cornerstone.services.scheduler.TimeScheduler" );
  +    /**
  +     * Set the configuration data for the handler
  +     *
  +     * @param theData configuration data for the handler
  +     */
  +    void setConfigurationData(NNTPHandlerConfigurationData theData) {
  +        theConfigData = theData;
  +    }
   
  -        repo = (NNTPRepository)componentManager
  -            .lookup("org.apache.james.nntpserver.repository.NNTPRepository");
  -        getLogger().debug("NNTPHandler compose...end");
  +    /**
  +     * Set the Watchdog for use by this handler.
  +     *
  +     * @param theWatchdog the watchdog
  +     */
  +    void setWatchdog(Watchdog theWatchdog) {
  +        this.theWatchdog = theWatchdog;
       }
   
       /**
  -     * @see org.apache.avalon.framework.configuration.Configurable#configure(Configuration)
  +     * Gets the Watchdog Target that should be used by Watchdogs managing
  +     * this connection.
  +     *
  +     * @return the WatchdogTarget
        */
  -    public void configure(Configuration configuration)
  -            throws ConfigurationException {
  -        super.configure(configuration);
  -        getLogger().debug("NNTPHandler configure...begin");
  -        authRequired =
  -            configuration.getChild("authRequired").getValueAsBoolean(false);
  -        getLogger().debug("NNTPHandler configure...end");
  +    WatchdogTarget getWatchdogTarget() {
  +        return theWatchdogTarget;
  +    }
  +
  +    /**
  +     * Idle out this connection
  +     */
  +    void idleClose() {
  +        if (getLogger() != null) {
  +            getLogger().error("Remote Manager Connection has idled out.");
  +        }
  +        try {
  +            if (socket != null) {
  +                socket.close();
  +            }
  +        } catch (Exception e) {
  +            // ignored
  +        }
       }
   
       /**
        * @see org.apache.avalon.cornerstone.services.connection.ConnectionHandler#handleConnection(Socket)
        */
       public void handleConnection( Socket connection ) throws IOException {
  +
           try {
               this.socket = connection;
  -            reader = new BufferedReader(new InputStreamReader(socket.getInputStream()));
  -            writer = new PrintWriter(socket.getOutputStream()) {
  -                // TODO: This implementation is sensitive to details of the PrintWriter
  -                //       implementation.  Specifically, that println(String) calls println().
  -                //       This should be corrected so that this sensitivity is removed.
  -                    public void println() {
  -                        // lines must end with CRLF, irrespective of the OS
  -                        print("\r\n");
  -                        flush();
  -                    }
  -                    public void println(String s) {
  -                        super.println(s);
  -                        if (getLogger().isDebugEnabled()) {
  -                            getLogger().debug("Sent: " + s);
  -                        }
  -                    }
  -                };
  +            reader = new BufferedReader(new InputStreamReader(socket.getInputStream(), "ASCII"), 1024);
  +            writer = new InternetPrintWriter(new BufferedWriter(new OutputStreamWriter(socket.getOutputStream()), 1024), true);
           } catch (Exception e) {
               getLogger().error( "Cannot open connection from: " + e.getMessage(), e );
           }
   
  -        String triggerId = this.toString();
  -
           try {
  -            final PeriodicTimeTrigger trigger = new PeriodicTimeTrigger( timeout, -1 );
  -            scheduler.addTrigger( triggerId, trigger, this );
  -
               // section 7.1
  -            if ( repo.isReadOnly() ) {
  +            if ( theConfigData.getNNTPRepository().isReadOnly() ) {
                   StringBuffer respBuffer =
                       new StringBuffer(128)
                           .append("201 ")
  -                        .append(helloName)
  +                        .append(theConfigData.getHelloName())
                           .append(" NNTP Service Ready, posting prohibited");
                   writer.println(respBuffer.toString());
               } else {
                   StringBuffer respBuffer =
                       new StringBuffer(128)
                               .append("200 ")
  -                            .append(helloName)
  +                            .append(theConfigData.getHelloName())
                               .append(" NNTP Service Ready, posting permitted");
                   writer.println(respBuffer.toString());
               }
   
  +            theWatchdog.start();
               while (parseCommand(reader.readLine())) {
  -                scheduler.resetTrigger(triggerId);
  +                theWatchdog.reset();
               }
  +            theWatchdog.stop();
   
               getLogger().info("Connection closed");
           } catch (Exception e) {
               doQUIT(null);
               getLogger().error( "Exception during connection:" + e.getMessage(), e );
           } finally {
  -            try {
  -                if (reader != null) {
  -                    reader.close();
  -                }
  -            } catch (IOException ioe) {
  -                getLogger().warn("NNTPHandler: Unexpected exception occurred while closing reader: " + ioe);
  +            resetHandler();
  +        }
  +    }
  +
  +    /**
  +     * Resets the handler data to a basic state.
  +     */
  +    private void resetHandler() {
  +
  +        // Clear the Watchdog
  +        if (theWatchdog != null) {
  +            if (theWatchdog instanceof Disposable) {
  +                ((Disposable)theWatchdog).dispose();
               }
  +            theWatchdog = null;
  +        }
   
  -            if (writer != null) {
  -                writer.close();
  +        // Clear the streams
  +        try {
  +            if (reader != null) {
  +                reader.close();
               }
  +        } catch (IOException ioe) {
  +            getLogger().warn("NNTPHandler: Unexpected exception occurred while closing reader: " + ioe);
  +        } finally {
  +            reader = null;
  +        }
   
  -            scheduler.removeTrigger(triggerId);
  +        if (writer != null) {
  +            writer.close();
  +            writer = null;
  +        }
   
  -            try {
  -                if (socket != null) {
  -                    socket.close();
  -                }
  -            } catch (IOException ioe) {
  -                getLogger().warn("NNTPHandler: Unexpected exception occurred while closing socket: " + ioe);
  +        try {
  +            if (socket != null) {
  +                socket.close();
               }
  +        } catch (IOException ioe) {
  +            getLogger().warn("NNTPHandler: Unexpected exception occurred while closing socket: " + ioe);
  +        } finally {
  +            socket = null;
           }
  -    }
   
  -    /**
  -     * Callback method called when the the PeriodicTimeTrigger in 
  -     * handleConnection is triggered.  In this case the trigger is
  -     * being used as a timeout, so the method simply closes the connection.
  -     *
  -     * @param triggerName the name of the trigger
  -     */
  -    public void targetTriggered( final String triggerName ) {
  -        getLogger().error("Connection timeout on socket");
  -        try {
  -            writer.println("Connection timeout. Closing connection");
  -            socket.close();
  -        } catch (IOException e) { }
  +        // Clear the selected group, article info
  +        group = null;
  +        currentArticleNumber = -1;
  +
  +        // Clear the authentication info
  +        user = null;
  +        password = null;
  +
  +        // Clear the config data
  +        theConfigData = null;
       }
   
       /**
  @@ -281,57 +429,57 @@
               getLogger().debug("Command not allowed.");
               return true;
           }
  -        if ((command.equals("MODE")) && (argument != null) &&
  -            argument.toUpperCase(Locale.US).equals("READER")) {
  +        if ((command.equals(COMMAND_MODE)) && (argument != null) &&
  +            argument.toUpperCase(Locale.US).equals(MODE_TYPE_READER)) {
               doMODEREADER(argument);
  -        } else if ( command.equals("LIST")) {
  +        } else if ( command.equals(COMMAND_LIST)) {
               doLIST(argument);
  -        } else if ( command.equals("GROUP") ) {
  +        } else if ( command.equals(COMMAND_GROUP) ) {
               doGROUP(argument);
  -        } else if ( command.equals("NEXT") ) {
  +        } else if ( command.equals(COMMAND_NEXT) ) {
               doNEXT(argument);
  -        } else if ( command.equals("LAST") ) {
  +        } else if ( command.equals(COMMAND_LAST) ) {
               doLAST(argument);
  -        } else if ( command.equals("ARTICLE") ) {
  +        } else if ( command.equals(COMMAND_ARTICLE) ) {
               doARTICLE(argument);
  -        } else if ( command.equals("HEAD") ) {
  +        } else if ( command.equals(COMMAND_HEAD) ) {
               doHEAD(argument);
  -        } else if ( command.equals("BODY") ) {
  +        } else if ( command.equals(COMMAND_BODY) ) {
               doBODY(argument);
  -        } else if ( command.equals("STAT") ) {
  +        } else if ( command.equals(COMMAND_STAT) ) {
               doSTAT(argument);
  -        } else if ( command.equals("POST") ) {
  +        } else if ( command.equals(COMMAND_POST) ) {
               doPOST(argument);
  -        } else if ( command.equals("IHAVE") ) {
  +        } else if ( command.equals(COMMAND_IHAVE) ) {
               doIHAVE(argument);
  -        } else if ( command.equals("QUIT") ) {
  +        } else if ( command.equals(COMMAND_QUIT) ) {
               doQUIT(argument);
  -        } else if ( command.equals("DATE") ) {
  +        } else if ( command.equals(COMMAND_DATE) ) {
               doDATE(argument);
  -        } else if ( command.equals("HELP") ) {
  +        } else if ( command.equals(COMMAND_HELP) ) {
               doHELP(argument);
  -        } else if ( command.equals("NEWGROUPS") ) {
  +        } else if ( command.equals(COMMAND_NEWGROUPS) ) {
               doNEWGROUPS(argument);
  -        } else if ( command.equals("NEWNEWS") ) {
  +        } else if ( command.equals(COMMAND_NEWNEWS) ) {
               doNEWNEWS(argument);
  -        } else if ( command.equals("LISTGROUP") ) {
  +        } else if ( command.equals(COMMAND_LISTGROUP) ) {
               doLISTGROUP(argument);
  -        } else if ( command.equals("OVER") ) {
  +        } else if ( command.equals(COMMAND_OVER) ) {
               doOVER(argument);
  -        } else if ( command.equals("XOVER") ) {
  +        } else if ( command.equals(COMMAND_XOVER) ) {
               doXOVER(argument);
  -        } else if ( command.equals("PAT") ) {
  -            doPAT(argument);
  -        } else if ( command.equals("HDR") ) {
  +        } else if ( command.equals(COMMAND_HDR) ) {
               doHDR(argument);
  -        } else if ( command.equals("XHDR") ) {
  +        } else if ( command.equals(COMMAND_XHDR) ) {
               doXHDR(argument);
  -        } else if ( command.equals("AUTHINFO") ) {
  +        } else if ( command.equals(COMMAND_AUTHINFO) ) {
               doAUTHINFO(argument);
  +        } else if ( command.equals("PAT") ) {
  +            doPAT(argument);
           } else {
               doUnknownCommand(command, argument);
           }
  -        return (command.equals("QUIT") == false);
  +        return (command.equals(COMMAND_QUIT) == false);
       }
   
       /**
  @@ -350,7 +498,7 @@
                       .append(argument);
               getLogger().debug(logBuffer.toString());
           }
  -        writer.println("501 Syntax error");
  +        writer.println("500 Unknown command");
       }
   
       /**
  @@ -374,10 +522,10 @@
               writer.println("501 Syntax error");
               return;
           }
  -        if ( command.equals("USER") ) {
  +        if ( command.equals(AUTHINFO_PARAM_USER) ) {
               user = value;
               writer.println("381 More authentication information required");
  -        } else if ( command.equals("PASS") ) {
  +        } else if ( command.equals(AUTHINFO_PARAM_PASS) ) {
               password = value;
               if ( isAuthenticated() ) {
                   writer.println("281 Authentication accepted");
  @@ -399,8 +547,16 @@
        */
       private void doNEWNEWS(String argument) {
           // see section 11.4
  +        Date theDate = null;
  +        try {
  +            theDate = getDateFrom(argument);
  +        } catch (NNTPException nntpe) {
  +            getLogger().error("NEWNEWS had an invalid argument", nntpe);
  +            writer.println("501 Syntax error");
  +            return;
  +        }
  +        Iterator iter = theConfigData.getNNTPRepository().getArticlesSince(theDate);
           writer.println("230 list of new articles by message-id follows");
  -        Iterator iter = repo.getArticlesSince(getDateFrom(argument));
           while ( iter.hasNext() ) {
               StringBuffer iterBuffer =
                   new StringBuffer(64)
  @@ -421,26 +577,35 @@
        */
       private void doNEWGROUPS(String argument) {
           // see section 11.3
  -        // there seeem to be few differences.
  -        // draft-ietf-nntpext-base-15.txt mentions 231 in section 11.3.1, 
  -        // but examples have response code 230. rfc977 has 231 response code.
           // both draft-ietf-nntpext-base-15.txt and rfc977 have only group names 
           // in response lines, but INN sends 
           // '<group name> <last article> <first article> <posting allowed>'
           // NOTE: following INN over either document.
  +        //
  +        // TODO: Check this.  Audit at http://www.academ.com/pipermail/ietf-nntp/2001-July/002185.html
  +        // doesn't mention the supposed discrepancy.  Consider changing code to 
  +        // be in line with spec.
  +        Date theDate = null;
  +        try {
  +            theDate = getDateFrom(argument);
  +        } catch (NNTPException nntpe) {
  +            getLogger().error("NEWGROUPS had an invalid argument", nntpe);
  +            writer.println("501 Syntax error");
  +            return;
  +        }
  +        Iterator iter = theConfigData.getNNTPRepository().getGroupsSince(theDate);
           writer.println("231 list of new newsgroups follows");
  -        Iterator iter = repo.getGroupsSince(getDateFrom(argument));
           while ( iter.hasNext() ) {
  -            NNTPGroup group = (NNTPGroup)iter.next();
  +            NNTPGroup currentGroup = (NNTPGroup)iter.next();
               StringBuffer iterBuffer =
                   new StringBuffer(128)
  -                    .append(group.getName())
  +                    .append(currentGroup.getName())
                       .append(" ")
  -                    .append(group.getLastArticleNumber())
  +                    .append(currentGroup.getLastArticleNumber())
                       .append(" ")
  -                    .append(group.getFirstArticleNumber())
  +                    .append(currentGroup.getFirstArticleNumber())
                       .append(" ")
  -                    .append((group.isPostAllowed()?"y":"n"));
  +                    .append((currentGroup.isPostAllowed()?"y":"n"));
               writer.println(iterBuffer.toString());
           }
           writer.println(".");
  @@ -462,8 +627,6 @@
        * @param argument the argument passed in with the DATE command
        */
       private void doDATE(String argument) {
  -        //Calendar c = Calendar.getInstance();
  -        //long UTC_OFFSET = c.get(c.ZONE_OFFSET) + c.get(c.DST_OFFSET);
           Date dt = new Date(System.currentTimeMillis()-UTC_OFFSET);
           String dtStr = DF_RFC2980.format(new Date(dt.getTime() - UTC_OFFSET));
           writer.println("111 "+dtStr);
  @@ -528,7 +691,7 @@
               }
           }
   
  -        Iterator iter = repo.getMatchedGroups(wildmat);
  +        Iterator iter = theConfigData.getNNTPRepository().getMatchedGroups(wildmat);
           writer.println("215 list of newsgroups follows");
           while ( iter.hasNext() ) {
               NNTPGroup theGroup = (NNTPGroup)iter.next();
  @@ -549,11 +712,11 @@
        */
       private void doIHAVE(String id) {
           // see section 9.3.2.1
  -        if (!(id.startsWith("<") && id.endsWith(">"))) {
  +        if (!isMessageId(id)) {
               writer.println("501 command syntax error");
               return;
           }
  -        NNTPArticle article = repo.getArticleFromID(id);
  +        NNTPArticle article = theConfigData.getNNTPRepository().getArticleFromID(id);
           if ( article != null ) {
               writer.println("435 article not wanted - do not send it");
           } else {
  @@ -566,41 +729,150 @@
       /**
        * Posts an article to the news server.
        *
  -     * @param argument the argument passed in with the AUTHINFO command
  +     * @param argument the argument passed in with the POST command
        */
       private void doPOST(String argument) {
           // see section 9.3.1.1
  +        if ( argument != null ) {
  +            writer.println("501 Syntax error - unexpected parameter");
  +        }
           writer.println("340 send article to be posted. End with <CR-LF>.<CR-LF>");
           createArticle();
           writer.println("240 article received ok");
       }
   
       /**
  -     * Sets the current article pointer.
  +     * Executes the STAT command.  Sets the current article pointer,
  +     * returns article information.
        *
  -     * @param the argument passed in to the HEAD command,
  +     * @param the argument passed in to the STAT command,
        *        which should be an article number or message id.
        *        If no parameter is provided, the current selected
        *        article is used.
        */
       private void doSTAT(String param) {
  -        doARTICLE(param,ArticleWriter.Factory.STAT(writer));
  +        // section 9.2.4
  +        NNTPArticle article = null;
  +        if (isMessageId(param)) {
  +            article = theConfigData.getNNTPRepository().getArticleFromID(param);
  +            if ( article == null ) {
  +                writer.println("430 no such article");
  +                return;
  +            } else {
  +                StringBuffer respBuffer =
  +                    new StringBuffer(64)
  +                            .append("223 0 ")
  +                            .append(param);
  +                writer.println(respBuffer.toString());
  +            }
  +        } else {
  +            int newArticleNumber = currentArticleNumber;
  +            if ( group == null ) {
  +                writer.println("412 no newsgroup selected");
  +                return;
  +            } else {
  +                if ( param == null ) {
  +                    if ( currentArticleNumber < 0 ) {
  +                        writer.println("420 no current article selected");
  +                        return;
  +                    } else {
  +                        article = group.getArticle(currentArticleNumber);
  +                    }
  +                }
  +                else {
  +                    newArticleNumber = Integer.parseInt(param);
  +                    article = group.getArticle(newArticleNumber);
  +                }
  +                if ( article == null ) {
  +                    writer.println("423 no such article number in this group");
  +                    return;
  +                } else {
  +                    currentArticleNumber = newArticleNumber;
  +                    String articleID = article.getUniqueID();
  +                    if (articleID == null) {
  +                        articleID = "<0>";
  +                    }
  +                    StringBuffer respBuffer =
  +                        new StringBuffer(128)
  +                                .append("223 ")
  +                                .append(article.getArticleNumber())
  +                                .append(" ")
  +                                .append(articleID);
  +                    writer.println(respBuffer.toString());
  +                }
  +            }
  +        }
       }
   
       /**
  -     * Displays the body of a particular article.
  +     * Executes the BODY command.  Sets the current article pointer,
  +     * returns article information and body.
        *
  -     * @param the argument passed in to the HEAD command,
  +     * @param the argument passed in to the BODY command,
        *        which should be an article number or message id.
        *        If no parameter is provided, the current selected
        *        article is used.
        */
       private void doBODY(String param) {
  -        doARTICLE(param,ArticleWriter.Factory.BODY(writer));
  +        // section 9.2.3
  +        NNTPArticle article = null;
  +        if (isMessageId(param)) {
  +            article = theConfigData.getNNTPRepository().getArticleFromID(param);
  +            if ( article == null ) {
  +                writer.println("430 no such article");
  +                return;
  +            } else {
  +                StringBuffer respBuffer =
  +                    new StringBuffer(64)
  +                            .append("222 0 ")
  +                            .append(param);
  +                writer.println(respBuffer.toString());
  +            }
  +        } else {
  +            int newArticleNumber = currentArticleNumber;
  +            if ( group == null ) {
  +                writer.println("412 no newsgroup selected");
  +                return;
  +            } else {
  +                if ( param == null ) {
  +                    if ( currentArticleNumber < 0 ) {
  +                        writer.println("420 no current article selected");
  +                        return;
  +                    } else {
  +                        article = group.getArticle(currentArticleNumber);
  +                    }
  +                }
  +                else {
  +                    newArticleNumber = Integer.parseInt(param);
  +                    article = group.getArticle(newArticleNumber);
  +                }
  +                if ( article == null ) {
  +                    writer.println("423 no such article number in this group");
  +                } else {
  +                    currentArticleNumber = newArticleNumber;
  +                    String articleID = article.getUniqueID();
  +                    if (articleID == null) {
  +                        articleID = "<0>";
  +                    }
  +                    StringBuffer respBuffer =
  +                        new StringBuffer(128)
  +                                .append("222 ")
  +                                .append(article.getArticleNumber())
  +                                .append(" ")
  +                                .append(articleID);
  +                    writer.println(respBuffer.toString());
  +                }
  +            }
  +        }
  +        if (article != null) {
  +            article.writeBody(writer);
  +            writer.println(".");
  +        }
       }
   
       /**
  -     * Displays the header of a particular article.
  +     * Executes the HEAD command.  Sets the current article pointer,
  +     * returns article information and headers.
        *
        * @param the argument passed in to the HEAD command,
        *        which should be an article number or message id.
  @@ -608,78 +880,126 @@
        *        article is used.
        */
       private void doHEAD(String param) {
  -        doARTICLE(param,ArticleWriter.Factory.HEAD(writer));
  +        // section 9.2.2
  +        NNTPArticle article = null;
  +        if (isMessageId(param)) {
  +            article = theConfigData.getNNTPRepository().getArticleFromID(param);
  +            if ( article == null ) {
  +                writer.println("430 no such article");
  +                return;
  +            } else {
  +                StringBuffer respBuffer =
  +                    new StringBuffer(64)
  +                            .append("221 0 ")
  +                            .append(param);
  +                writer.println(respBuffer.toString());
  +            }
  +        } else {
  +            int newArticleNumber = currentArticleNumber;
  +            if ( group == null ) {
  +                writer.println("412 no newsgroup selected");
  +                return;
  +            } else {
  +                if ( param == null ) {
  +                    if ( currentArticleNumber < 0 ) {
  +                        writer.println("420 no current article selected");
  +                        return;
  +                    } else {
  +                        article = group.getArticle(currentArticleNumber);
  +                    }
  +                }
  +                else {
  +                    newArticleNumber = Integer.parseInt(param);
  +                    article = group.getArticle(newArticleNumber);
  +                }
  +                if ( article == null ) {
  +                    writer.println("423 no such article number in this group");
  +                } else {
  +                    currentArticleNumber = newArticleNumber;
  +                    String articleID = article.getUniqueID();
  +                    if (articleID == null) {
  +                        articleID = "<0>";
  +                    }
  +                    StringBuffer respBuffer =
  +                        new StringBuffer(128)
  +                                .append("221 ")
  +                                .append(article.getArticleNumber())
  +                                .append(" ")
  +                                .append(articleID);
  +                    writer.println(respBuffer.toString());
  +                }
  +            }
  +        }
  +        if (article != null) {
  +            article.writeHead(writer);
  +            writer.println(".");
  +        }
       }
   
       /**
  -     * Displays the header and body of a particular article.
  +     * Executes the ARTICLE command.  Sets the current article pointer,
  +     * returns article information and contents.
        *
  -     * @param the argument passed in to the HEAD command,
  +     * @param the argument passed in to the ARTICLE command,
        *        which should be an article number or message id.
        *        If no parameter is provided, the current selected
        *        article is used.
        */
       private void doARTICLE(String param) {
  -        doARTICLE(param,ArticleWriter.Factory.ARTICLE(writer));
  -    }
  -
  -    /**
  -     * The implementation of the method body for the ARTICLE, STAT,
  -     * HEAD, and BODY commands.
  -     *
  -     * @param the argument passed in to the command,
  -     *        which should be an article number or message id.
  -     *        If no parameter is provided, the current selected
  -     *        article is used.
  -     * @param articleWriter the writer that determines the output
  -     *                      written to the client.
  -     */
  -    private void doARTICLE(String param,ArticleWriter articleWriter) {
           // section 9.2.1
           NNTPArticle article = null;
  -        if ( (param != null) && param.startsWith("<") && param.endsWith(">") ) {
  -            article = repo.getArticleFromID(param);
  +        if (isMessageId(param)) {
  +            article = theConfigData.getNNTPRepository().getArticleFromID(param);
               if ( article == null ) {
                   writer.println("430 no such article");
  +                return;
               } else {
                   StringBuffer respBuffer =
                       new StringBuffer(64)
                               .append("220 0 ")
  -                            .append(param)
  -                            .append(" article retrieved and follows");
  +                            .append(param);
                   writer.println(respBuffer.toString());
               }
           } else {
  +            int newArticleNumber = currentArticleNumber;
               if ( group == null ) {
                   writer.println("412 no newsgroup selected");
  +                return;
               } else {
                   if ( param == null ) {
  -                    if ( group.getCurrentArticleNumber() < 0 ) {
  +                    if ( currentArticleNumber < 0 ) {
                           writer.println("420 no current article selected");
  +                        return;
                       } else {
  -                        article = group.getCurrentArticle();
  +                        article = group.getArticle(currentArticleNumber);
                       }
                   }
                   else {
  -                    article = group.getArticle(Integer.parseInt(param));
  +                    newArticleNumber = Integer.parseInt(param);
  +                    article = group.getArticle(newArticleNumber);
                   }
                   if ( article == null ) {
                       writer.println("423 no such article number in this group");
                   } else {
  +                    currentArticleNumber = newArticleNumber;
  +                    String articleID = article.getUniqueID();
  +                    if (articleID == null) {
  +                        articleID = "<0>";
  +                    }
                       StringBuffer respBuffer =
                           new StringBuffer(128)
                                   .append("220 ")
                                   .append(article.getArticleNumber())
                                   .append(" ")
  -                                .append(article.getUniqueID())
  -                                .append(" article retrieved and follows");
  +                                .append(articleID);
                       writer.println(respBuffer.toString());
                   }
               }
           }
  -        if ( article != null ) {
  -            articleWriter.write(article);
  -        }   
  +        if (article != null) {
  +            article.writeArticle(writer);
  +            writer.println(".");
  +        }
       }
   
       /**
  @@ -689,15 +1009,17 @@
        */
       private void doNEXT(String argument) {
           // section 9.1.1.3.1
  -        if ( group == null ) {
  +        if ( argument != null ) {
  +            writer.println("501 Syntax error - unexpected parameter");
  +        } else if ( group == null ) {
               writer.println("412 no newsgroup selected");
  -        } else if ( group.getCurrentArticleNumber() < 0 ) {
  +        } else if ( currentArticleNumber < 0 ) {
               writer.println("420 no current article has been selected");
  -        } else if ( group.getCurrentArticleNumber() >= group.getLastArticleNumber() ) {
  +        } else if ( currentArticleNumber >= group.getLastArticleNumber() ) {
               writer.println("421 no next article in this group");
           } else {
  -            group.setCurrentArticleNumber(group.getCurrentArticleNumber()+1);
  -            NNTPArticle article = group.getCurrentArticle();
  +            currentArticleNumber++;
  +            NNTPArticle article = group.getArticle(currentArticleNumber);
               StringBuffer respBuffer =
                   new StringBuffer(64)
                           .append("223 ")
  @@ -709,22 +1031,24 @@
       }
   
       /**
  -     * Advances the currently selected article pointer to the last article
  -     * in the selected group.
  +     * Moves the currently selected article pointer to the article
  +     * previous to the currently selected article in the selected group.
        *
        * @param argument the argument passed in with the LAST command
        */
       private void doLAST(String argument) {
           // section 9.1.1.2.1
  -        if ( group == null ) {
  +        if ( argument != null ) {
  +            writer.println("501 Syntax error - unexpected parameter");
  +        } else if ( group == null ) {
               writer.println("412 no newsgroup selected");
  -        } else if ( group.getCurrentArticleNumber() < 0 ) {
  +        } else if ( currentArticleNumber < 0 ) {
               writer.println("420 no current article has been selected");
  -        } else if ( group.getCurrentArticleNumber() <= group.getFirstArticleNumber() ) {
  +        } else if ( currentArticleNumber <= group.getFirstArticleNumber() ) {
               writer.println("422 no previous article in this group");
           } else {
  -            group.setCurrentArticleNumber(group.getCurrentArticleNumber()-1);
  -            NNTPArticle article = group.getCurrentArticle();
  +            currentArticleNumber--;
  +            NNTPArticle article = group.getArticle(currentArticleNumber);
               StringBuffer respBuffer =
                   new StringBuffer(64)
                           .append("223 ")
  @@ -741,7 +1065,11 @@
        * @param group the name of the group being selected.
        */
       private void doGROUP(String groupName) {
  -        NNTPGroup newGroup = repo.getGroup(groupName);
  +        if (groupName == null) {
  +            writer.println("501 Syntax error - missing required parameter");
  +            return;
  +        }
  +        NNTPGroup newGroup = theConfigData.getNNTPRepository().getGroup(groupName);
           // section 9.1.1.1
           if ( newGroup == null ) {
               writer.println("411 no such newsgroup");
  @@ -755,6 +1083,15 @@
               int articleCount = group.getNumberOfArticles();
               int lowWaterMark = group.getFirstArticleNumber();
               int highWaterMark = group.getLastArticleNumber();
  +
  +            // Set the current article pointer.  If no
  +            // articles are in the group, the current article
  +            // pointer should be explicitly unset.
  +            if (articleCount != 0) {
  +                currentArticleNumber = lowWaterMark;
  +            } else {
  +                currentArticleNumber = -1;
  +            }
               StringBuffer respBuffer =
                   new StringBuffer(128)
                           .append("211 ")
  @@ -792,7 +1129,10 @@
        */
       private void doMODEREADER(String argument) {
           // 7.2
  -        writer.println(repo.isReadOnly()
  +        if ( argument != null ) {
  +            writer.println("501 Syntax error - unexpected parameter");
  +        }
  +        writer.println(theConfigData.getNNTPRepository().isReadOnly()
                          ? "201 Posting Not Permitted" : "200 Posting Permitted");
       }
   
  @@ -806,18 +1146,29 @@
           // 9.5.1.1.1
           if (groupName==null) {
               if ( group == null) {
  -                writer.println("412 not currently in newsgroup");
  +                writer.println("412 no news group currently selected");
                   return;
               }
           }
           else {
  -            group = repo.getGroup(groupName);
  +            group = theConfigData.getNNTPRepository().getGroup(groupName);
               if ( group == null ) {
                   writer.println("411 no such newsgroup");
                   return;
               }
           }
           if ( group != null ) {
  +            this.group = group;
  +
  +            // Set the current article pointer.  If no
  +            // articles are in the group, the current article
  +            // pointer should be explicitly unset.
  +            if (group.getNumberOfArticles() > 0) {
  +                currentArticleNumber = group.getFirstArticleNumber();
  +            } else {
  +                currentArticleNumber = -1;
  +            }
  +
               writer.println("211 list of article numbers follow");
   
               Iterator iter = group.getArticles();
  @@ -826,8 +1177,6 @@
                   writer.println(article.getArticleNumber());
               }
               writer.println(".");
  -            this.group = group;
  -            group.setCurrentArticleNumber(group.getFirstArticleNumber());
           }
       }
   
  @@ -836,8 +1185,12 @@
        */
       private void doLISTOVERVIEWFMT() {
           // 9.5.3.1.1
  -        // 503 means information is not available as per 9.5.2.1
  -        writer.println("503 program error, function not performed");
  +        writer.println("215 Information follows");
  +        String[] overviewHeaders = theConfigData.getNNTPRepository().getOverviewFormat();
  +        for (int i = 0;  i < overviewHeaders.length; i++) {
  +            writer.println(overviewHeaders[i]);
  +        }
  +        writer.println(".");
       }
   
       /**
  @@ -868,13 +1221,24 @@
        */
       private void doHDR(String argument) {
           // 9.5.3
  +        if (argument == null) {
  +            writer.println("501 Syntax error - missing required parameter");
  +        }
           String hdr = argument;
           String range = null;
  -        int spaceIndex = argument.indexOf(" ");
  +        int spaceIndex = hdr.indexOf(" ");
           if (spaceIndex >= 0 ) {
               range = hdr.substring(spaceIndex + 1);
               hdr = hdr.substring(0, spaceIndex);
           }
  +        if (group == null ) {
  +            writer.println("412 No news group currently selected.");
  +            return;
  +        }
  +        if ((range == null) && (currentArticleNumber < 0)) {
  +            writer.println("420 No current article selected");
  +            return;
  +        }
           NNTPArticle[] article = getRange(range);
           if ( article == null ) {
               writer.println("412 no newsgroup selected");
  @@ -920,14 +1284,23 @@
               writer.println("412 No newsgroup selected");
               return;
           }
  +        if ((range == null) && (currentArticleNumber < 0)) {
  +            writer.println("420 No current article selected");
  +            return;
  +        }
           NNTPArticle[] article = getRange(range);
  -        ArticleWriter articleWriter = ArticleWriter.Factory.OVER(writer);
           if ( article.length == 0 ) {
               writer.println("420 No article(s) selected");
           } else {
               writer.println("224 Overview information follows");
               for ( int i = 0 ; i < article.length ; i++ ) {
  -                articleWriter.write(article[i]);
  +                article[i].writeOverview(writer);
  +                if (i % 100 == 0) {
  +                    // Reset the watchdog ever hundred headers or so
  +                    // to ensure the connection doesn't timeout for slow
  +                    // clients
  +                    theWatchdog.reset();
  +                }
               }
               writer.println(".");
           }
  @@ -937,7 +1310,7 @@
        * Handles the transaction for getting the article data.
        */
       private void createArticle() {
  -        repo.createArticle(new NNTPLineReaderImpl(reader));
  +        theConfigData.getNNTPRepository().createArticle(new NNTPLineReaderImpl(reader));
       }
   
       /**
  @@ -949,7 +1322,13 @@
        * @param argument the date string
        */
       private Date getDateFrom(String argument) {
  +        if (argument == null) {
  +            throw new NNTPException("Date argument was absent.");
  +        }
           StringTokenizer tok = new StringTokenizer(argument, " ");
  +        if (tok.countTokens() < 2) {
  +            throw new NNTPException("Date argument was ill-formed.");
  +        }
           String date = tok.nextToken();
           String time = tok.nextToken();
           boolean utc = ( tok.hasMoreTokens() );
  @@ -968,7 +1347,7 @@
           } catch ( ParseException pe ) {
               StringBuffer exceptionBuffer =
                   new StringBuffer(128)
  -                    .append("date extraction failed: ")
  +                    .append("Date extraction failed: ")
                       .append(date)
                       .append(",")
                       .append(time)
  @@ -980,35 +1359,38 @@
   
       /**
        * Returns the list of articles that match the range.
  +     *
  +     * A precondition of this method is that the selected
  +     * group be non-null.  The current article pointer must
  +     * be valid if no range is explicitly provided.
  +     *
        * @return null indicates insufficient information to
        * fetch the list of articles
        */
       private NNTPArticle[] getRange(String range) {
           // check for msg id
  -        if ( range != null && range.startsWith("<") ) {
  -            NNTPArticle article = repo.getArticleFromID(range);
  +        if ( isMessageId(range)) {
  +            NNTPArticle article = theConfigData.getNNTPRepository().getArticleFromID(range);
               return ( article == null )
                   ? new NNTPArticle[0] : new NNTPArticle[] { article };
           }
   
  -        if ( group == null ) {
  -            return null;
  -        }
           if ( range == null ) {
  -            range = "" + group.getCurrentArticleNumber();
  +            range = "" + currentArticleNumber;
           }
   
           int start = -1;
           int end = -1;
           int idx = range.indexOf('-');
           if ( idx == -1 ) {
  -            start = end = Integer.parseInt(range);
  +            start = Integer.parseInt(range);
  +            end = start;
           } else {
               start = Integer.parseInt(range.substring(0,idx));
  -            if ( idx+1 == range.length() ) {
  +            if ( (idx + 1) == range.length() ) {
                   end = group.getLastArticleNumber();
               } else {
  -                end = Integer.parseInt(range.substring(idx+1));
  +                end = Integer.parseInt(range.substring(idx + 1));
               }
           }
           List list = new ArrayList();
  @@ -1046,9 +1428,9 @@
        * @return whether the connection has been authenticated.
        */
       private boolean isAuthenticated() {
  -        if ( authRequired ) {
  -            if  ((user != null) && (password != null) && (userRepository != null)) {
  -                return userRepository.test(user,password);
  +        if ( theConfigData.isAuthRequired() ) {
  +            if  ((user != null) && (password != null) && (theConfigData.getUsersRepository() != null)) {
  +                return theConfigData.getUsersRepository().test(user,password);
               } else {
                   return false;
               }
  @@ -1057,4 +1439,38 @@
           }
       }
   
  +    /**
  +     * Tests a string to see whether it is formatted as a message
  +     * ID.
  +     *
  +     * @param testString the string to test
  +     *
  +     * @return whether the string is a candidate message ID
  +     */
  +    private static boolean isMessageId(String testString) {
  +        if ((testString != null) &&
  +            (testString.startsWith("<")) &&
  +            (testString.endsWith(">"))) {
  +            return true;
  +        } else {
  +            return false;
  +        }
  +   }
  +
  +    /**
  +     * A private inner class which serves as an adaptor
  +     * between the WatchdogTarget interface and this
  +     * handler class.
  +     */
  +    private class NNTPWatchdogTarget
  +        implements WatchdogTarget {
  +
  +        /**
  +         * @see org.apache.james.util.watchdog.WatchdogTarget#execute()
  +         */
  +        public void execute() {
  +            NNTPHandler.this.idleClose();
  +        }
  +
  +    }
   }
  
  
  
  1.12      +199 -39   jakarta-james/src/java/org/apache/james/nntpserver/NNTPServer.java
  
  Index: NNTPServer.java
  ===================================================================
  RCS file: /home/cvs/jakarta-james/src/java/org/apache/james/nntpserver/NNTPServer.java,v
  retrieving revision 1.11
  retrieving revision 1.12
  diff -u -r1.11 -r1.12
  --- NNTPServer.java	2 Oct 2002 09:53:38 -0000	1.11
  +++ NNTPServer.java	26 Oct 2002 04:15:29 -0000	1.12
  @@ -6,52 +6,105 @@
    * the LICENSE file.
    */
   package org.apache.james.nntpserver;
  -import org.apache.avalon.cornerstone.services.connection.AbstractService;
  -import org.apache.avalon.cornerstone.services.connection.ConnectionHandlerFactory;
  -import org.apache.avalon.cornerstone.services.connection.DefaultHandlerFactory;
  +
  +import org.apache.avalon.cornerstone.services.connection.ConnectionHandler;
  +import org.apache.avalon.excalibur.pool.DefaultPool;
  +import org.apache.avalon.excalibur.pool.HardResourceLimitingPool;
  +import org.apache.avalon.excalibur.pool.ObjectFactory;
  +import org.apache.avalon.excalibur.pool.Pool;
  +import org.apache.avalon.excalibur.pool.Poolable;
  +import org.apache.avalon.framework.activity.Disposable;
  +import org.apache.avalon.framework.activity.Initializable;
   import org.apache.avalon.framework.configuration.Configuration;
   import org.apache.avalon.framework.configuration.ConfigurationException;
   import org.apache.avalon.framework.component.Component;
  +import org.apache.avalon.framework.component.ComponentException;
  +import org.apache.avalon.framework.component.ComponentManager;
  +import org.apache.avalon.framework.component.Composable;
  +import org.apache.avalon.framework.logger.LogEnabled;
  +
  +import org.apache.james.core.AbstractJamesService;
  +import org.apache.james.nntpserver.repository.NNTPRepository;
  +import org.apache.james.services.UsersRepository;
  +import org.apache.james.services.UsersStore;
  +import org.apache.james.util.watchdog.ThreadPerWatchdogFactory;
  +import org.apache.james.util.watchdog.Watchdog;
  +import org.apache.james.util.watchdog.WatchdogFactory;
  +import org.apache.james.util.watchdog.WatchdogTarget;
  +
   import java.net.InetAddress;
   import java.net.UnknownHostException;
  +
   /**
  - * NNTP Server Protocol Handler
  + * NNTP Server
    *
    * @author Harmeet Bedi <ha...@kodemuse.com>
    * @author  <a href="mailto:danny@apache.org">Danny Angus</a>
  + * @author Peter M. Goldstein <fa...@alum.mit.edu>
    */
  -public class NNTPServer extends AbstractService implements Component {
  +public class NNTPServer extends AbstractJamesService implements Component {
  +
  +    /**
  +     * Whether authentication is required to access this NNTP server
  +     */
  +    private boolean authRequired = false;
  +
  +    /**
  +     * The repository that stores the news articles for this NNTP server.
  +     */
  +    private NNTPRepository repo;
  +
  +    /**
  +     * The repository that stores the local users.  Used for authentication.
  +     */
  +    private UsersRepository userRepository = null;
  +
  +    /**
  +     * The pool used to provide NNTP Handler objects
  +     */
  +    private Pool theHandlerPool = null;
  +
  +    /**
  +     * The pool used to provide NNTP Handler objects
  +     */
  +    private ObjectFactory theHandlerFactory = new NNTPHandlerFactory();
   
       /**
  -     * Whether this service is enabled
  +     * The factory used to generate Watchdog objects
        */
  -    private volatile boolean enabled = true;
  +    private WatchdogFactory theWatchdogFactory;
   
  -    protected ConnectionHandlerFactory createFactory() {
  -        return new DefaultHandlerFactory(NNTPHandler.class);
  +    /**
  +     * The configuration data to be passed to the handler
  +     */
  +    private NNTPHandlerConfigurationData theConfigData
  +        = new NNTPHandlerConfigurationDataImpl();
  +
  +    /**
  +     * @see org.apache.avalon.framework.component.Composable#compose(ComponentManager)
  +     */
  +    public void compose( final ComponentManager componentManager )
  +        throws ComponentException {
  +        super.compose(componentManager);
  +        UsersStore usersStore = (UsersStore)componentManager.lookup(UsersStore.ROLE);
  +        userRepository = usersStore.getRepository("LocalUsers");
  +        if (userRepository == null) {
  +            throw new ComponentException("The user repository could not be found.");
  +        }
  +
  +        repo = (NNTPRepository)componentManager
  +            .lookup("org.apache.james.nntpserver.repository.NNTPRepository");
       }
   
       /**
        * @see org.apache.avalon.framework.configuration.Configurable#configure(Configuration)
        */
       public void configure(final Configuration configuration) throws ConfigurationException {
  -        enabled = configuration.getAttributeAsBoolean("enabled", true);
  -        if (enabled) {
  -            m_port = configuration.getChild("port").getValueAsInteger(119);
  -            try {
  -                String bindAddress = configuration.getChild("bind").getValue(null);
  -                if (null != bindAddress)
  -                    m_bindTo = InetAddress.getByName(bindAddress);
  -            } catch (final UnknownHostException unhe) {
  -                throw new ConfigurationException("Malformed bind parameter", unhe);
  -            }
  -            final boolean useTLS = configuration.getChild("useTLS").getValueAsBoolean(false);
  -            if (useTLS) {
  -                m_serverSocketType = "ssl";
  -            }
  -            super.configure(configuration.getChild("handler"));
  -            boolean authRequired =
  -                configuration.getChild("handler").getChild("authRequired").getValueAsBoolean(false);
  +        super.configure(configuration);
  +        if (isEnabled()) {
  +            Configuration handlerConfiguration = configuration.getChild("handler");
  +            authRequired =
  +                handlerConfiguration.getChild("authRequired").getValueAsBoolean(false);
               if (getLogger().isDebugEnabled()) {
                   if (authRequired) {
                       getLogger().debug("NNTP Server requires authentication.");
  @@ -59,9 +112,6 @@
                       getLogger().debug("NNTP Server doesn't require authentication.");
                   }
               }
  -            if (getLogger().isInfoEnabled()) {
  -                getLogger().info("Configured NNTPServer to run at : " + m_port);
  -            }
           }
       }
   
  @@ -69,22 +119,132 @@
        * @see org.apache.avalon.framework.activity.Initializable#initialize()
        */
       public void initialize() throws Exception {
  -        if (enabled) {
  -            super.initialize();
  -            System.out.println("NNTP Server Started " + m_connectionName);
  -            getLogger().info("NNTP Server Started " + m_connectionName);
  +        super.initialize();
  +        if (!isEnabled()) {
  +            return;
  +        }
  +
  +        if (connectionLimit != null) {
  +            theHandlerPool = new HardResourceLimitingPool(theHandlerFactory, 5, connectionLimit.intValue());
  +            getLogger().debug("Using a bounded pool for NNTP handlers with upper limit " + connectionLimit.intValue());
           } else {
  -            getLogger().info("NNTP Server Disabled");
  -            System.out.println("NNTP Server Disabled");
  +            // NOTE: The maximum here is not a real maximum.  The handler pool will continue to
  +            //       provide handlers beyond this value.
  +            theHandlerPool = new DefaultPool(theHandlerFactory, null, 5, 30);
  +            getLogger().debug("Using an unbounded pool for NNTP handlers.");
  +        }
  +        if (theHandlerPool instanceof LogEnabled) {
  +            ((LogEnabled)theHandlerPool).enableLogging(getLogger());
  +        }
  +        if (theHandlerPool instanceof Initializable) {
  +            ((Initializable)theHandlerPool).initialize();
  +        }
  +
  +        theWatchdogFactory = new ThreadPerWatchdogFactory(threadPool, timeout);
  +        if (theWatchdogFactory instanceof LogEnabled) {
  +            ((LogEnabled)theWatchdogFactory).enableLogging(getLogger());
           }
       }
   
       /**
  -     * @see org.apache.avalon.framework.activity.Disposable#dispose()
  +     * @see org.apache.james.core.AbstractJamesService#getDefaultPort()
        */
  -    public void dispose() {
  -        if (enabled) {
  -            super.dispose();
  +     protected int getDefaultPort() {
  +        return 119;
  +     }
  +
  +    /**
  +     * @see org.apache.james.core.AbstractJamesService#getServiceType()
  +     */
  +    public String getServiceType() {
  +        return "NNTP Service";
  +    }
  +
  +    /**
  +     * @see org.apache.avalon.cornerstone.services.connection.AbstractHandlerFactory#newHandler()
  +     */
  +    protected ConnectionHandler newHandler()
  +            throws Exception {
  +        NNTPHandler theHandler = (NNTPHandler)theHandlerPool.get();
  +
  +        Watchdog theWatchdog = theWatchdogFactory.getWatchdog(theHandler.getWatchdogTarget());
  +
  +        theHandler.setConfigurationData(theConfigData);
  +        theHandler.setWatchdog(theWatchdog);
  +        return theHandler;
  +    }
  +
  +    /**
  +     * @see org.apache.avalon.cornerstone.services.connection.ConnectionHandlerFactory#releaseConnectionHandler(ConnectionHandler)
  +     */
  +    public void releaseConnectionHandler( ConnectionHandler connectionHandler ) {
  +        if (!(connectionHandler instanceof NNTPHandler)) {
  +            throw new IllegalArgumentException("Attempted to return non-NNTPHandler to pool.");
  +        }
  +        theHandlerPool.put((Poolable)connectionHandler);
  +    }
  +
  +    /**
  +     * The factory for producing handlers.
  +     */
  +    private static class NNTPHandlerFactory
  +        implements ObjectFactory {
  +
  +        /**
  +         * @see org.apache.avalon.excalibur.pool.ObjectFactory#newInstance()
  +         */
  +        public Object newInstance() throws Exception {
  +            return new NNTPHandler();
           }
  +
  +        /**
  +         * @see org.apache.avalon.excalibur.pool.ObjectFactory#getCreatedClass()
  +         */
  +        public Class getCreatedClass() {
  +            return NNTPHandler.class;
  +        }
  +
  +        /**
  +         * @see org.apache.avalon.excalibur.pool.ObjectFactory#decommision(Object)
  +         */
  +        public void decommission( Object object ) throws Exception {
  +            return;
  +        }
  +    }
  +
  +    /**
  +     * A class to provide NNTP handler configuration to the handlers
  +     */
  +    private class NNTPHandlerConfigurationDataImpl
  +        implements NNTPHandlerConfigurationData {
  +
  +        /**
  +         * @see org.apache.james.smtpserver.NNTPHandlerConfigurationData#getHelloName()
  +         */
  +        public String getHelloName() {
  +            return NNTPServer.this.helloName;
  +        }
  +
  +        /**
  +         * @see org.apache.james.smtpserver.NNTPHandlerConfigurationData#isAuthRequired()
  +         */
  +        public boolean isAuthRequired() {
  +            return NNTPServer.this.authRequired;
  +        }
  +
  +        /**
  +         * @see org.apache.james.smtpserver.NNTPHandlerConfigurationData#getUsersRepository()
  +         */
  +        public UsersRepository getUsersRepository() {
  +            return NNTPServer.this.userRepository;
  +        }
  +
  +        /**
  +         * @see org.apache.james.smtpserver.NNTPHandlerConfigurationData#getNNTPRepository()
  +         */
  +        public NNTPRepository getNNTPRepository() {
  +            return NNTPServer.this.repo;
  +        }
  +
       }
   }
  
  
  
  1.6       +3 -3      jakarta-james/src/java/org/apache/james/nntpserver/NNTPServer.xinfo
  
  Index: NNTPServer.xinfo
  ===================================================================
  RCS file: /home/cvs/jakarta-james/src/java/org/apache/james/nntpserver/NNTPServer.xinfo,v
  retrieving revision 1.5
  retrieving revision 1.6
  diff -u -r1.5 -r1.6
  --- NNTPServer.xinfo	12 Oct 2002 19:32:54 -0000	1.5
  +++ NNTPServer.xinfo	26 Oct 2002 04:15:29 -0000	1.6
  @@ -20,14 +20,14 @@
         <service name="org.apache.avalon.cornerstone.services.sockets.SocketManager" version="1.0"/>
       </dependency>
       <dependency>
  -      <service name="org.apache.avalon.cornerstone.services.scheduler.TimeScheduler" version="1.0"/>
  -    </dependency> 
  -    <dependency>
         <service name="org.apache.james.nntpserver.repository.NNTPRepository" version="1.0"/>
       </dependency> 
       <dependency>
         <service name="org.apache.james.services.UsersStore" version="1.0"/>
       </dependency> 
  +    <dependency>
  +      <service name="org.apache.avalon.cornerstone.services.threads.ThreadManager" version="1.0"/>
  +    </dependency>
     </dependencies>
   
   </blockinfo>
  
  
  
  1.1                  jakarta-james/src/java/org/apache/james/nntpserver/NNTPHandlerConfigurationData.java
  
  Index: NNTPHandlerConfigurationData.java
  ===================================================================
  /*
   * Copyright (C) The Apache Software Foundation. All rights reserved.
   *
   * This software is published under the terms of the Apache Software License
   * version 1.1, a copy of which has been included with this distribution in
   * the LICENSE file.
   */
  package org.apache.james.nntpserver;
  
  import org.apache.james.nntpserver.repository.NNTPRepository;
  import org.apache.james.services.MailServer;
  import org.apache.james.services.UsersRepository;
  
  /**
   * Provides a number of server-wide constant values to the
   * NNTPHandlers
   *
   * @author Peter M. Goldstein <fa...@alum.mit.edu>
   */
  public interface NNTPHandlerConfigurationData {
  
      /**
       * Returns the service wide hello name
       *
       * @return the hello name
       */
      String getHelloName();
  
      /**
       * Returns whether NNTP auth is active for this server.
       *
       * @return whether NNTP authentication is on
       */
      boolean isAuthRequired();
  
      /**
       * Returns the NNTPRepository used by this service.
       *
       * @return the NNTPRepository used by this service
       */
      NNTPRepository getNNTPRepository();
  
      /**
       * Returns the UsersRepository for this service.
       *
       * @return the local users repository
       */
      UsersRepository getUsersRepository();
  
  }
  
  
  
  1.10      +1 -3      jakarta-james/src/java/org/apache/james/nntpserver/repository/NNTPArticleImpl.java
  
  Index: NNTPArticleImpl.java
  ===================================================================
  RCS file: /home/cvs/jakarta-james/src/java/org/apache/james/nntpserver/repository/NNTPArticleImpl.java,v
  retrieving revision 1.9
  retrieving revision 1.10
  diff -u -r1.9 -r1.10
  --- NNTPArticleImpl.java	4 Oct 2002 23:14:03 -0000	1.9
  +++ NNTPArticleImpl.java	26 Oct 2002 04:15:29 -0000	1.10
  @@ -133,7 +133,6 @@
               FileInputStream fin = new FileInputStream(articleFile);
               InternetHeaders hdr = new InternetHeaders(fin);
               fin.close();
  -            int articleNumber = getArticleNumber();
               String subject = hdr.getHeader("Subject",null);
               String author = hdr.getHeader("From",null);
               String date = hdr.getHeader("Date",null);
  @@ -142,12 +141,11 @@
               long byteCount = articleFile.length();
               long lineCount = -1;
               StringBuffer line=new StringBuffer(128)
  -                .append(articleNumber + "\t")
                   .append(cleanHeader(subject))    .append("\t")
                   .append(cleanHeader(author))     .append("\t")
                   .append(cleanHeader(date))       .append("\t")
                   .append(cleanHeader(msgId))      .append("\t")
  -                .append(cleanHeader(references)) .append("\t")         
  +                .append(cleanHeader(references)) .append("\t")
                   .append(byteCount + "\t")
                   .append(lineCount + "");
               prt.println(line.toString());
  
  
  
  1.5       +0 -22     jakarta-james/src/java/org/apache/james/nntpserver/repository/NNTPGroup.java
  
  Index: NNTPGroup.java
  ===================================================================
  RCS file: /home/cvs/jakarta-james/src/java/org/apache/james/nntpserver/repository/NNTPGroup.java,v
  retrieving revision 1.4
  retrieving revision 1.5
  diff -u -r1.4 -r1.5
  --- NNTPGroup.java	4 Oct 2002 08:17:49 -0000	1.4
  +++ NNTPGroup.java	26 Oct 2002 04:15:29 -0000	1.5
  @@ -41,21 +41,6 @@
       boolean isPostAllowed();
   
       /**
  -     * Returns the current article number.
  -     *
  -     * @return A return value of less than zero 
  -     *         indicates invalid/unknown value
  -     */
  -    int getCurrentArticleNumber();
  -
  -    /**
  -     * Sets the current article number.
  -     *
  -     * @param the new current article number
  -     */
  -    void setCurrentArticleNumber(int articleNumber);
  -
  -    /**
        * Gets the number of articles in the group.
        *
        * @return the number of articles in the group.
  @@ -75,13 +60,6 @@
        * @return the last article number in the group.
        */
       int getLastArticleNumber();
  -
  -    /**
  -     * Gets the current article.
  -     *
  -     * @return the current article
  -     */
  -    NNTPArticle getCurrentArticle();
   
       /**
        * Gets the article with the specified article number.
  
  
  
  1.9       +2 -31     jakarta-james/src/java/org/apache/james/nntpserver/repository/NNTPGroupImpl.java
  
  Index: NNTPGroupImpl.java
  ===================================================================
  RCS file: /home/cvs/jakarta-james/src/java/org/apache/james/nntpserver/repository/NNTPGroupImpl.java,v
  retrieving revision 1.8
  retrieving revision 1.9
  diff -u -r1.8 -r1.9
  --- NNTPGroupImpl.java	4 Oct 2002 23:14:03 -0000	1.8
  +++ NNTPGroupImpl.java	26 Oct 2002 04:15:29 -0000	1.9
  @@ -37,11 +37,6 @@
       private final File root;
   
       /**
  -     * The article number of the current article
  -     */
  -    private int currentArticle = -1;
  -
  -    /**
        * The last article number in the group
        */
       private int lastArticle;
  @@ -144,31 +139,6 @@
       }
   
       /**
  -     * @see org.apache.james.nntpserver.NNTPGroup#getCurrentArticleNumber()
  -     */
  -    public int getCurrentArticleNumber() {
  -        collectArticleRangeInfo();
  -        // this is not as per RFC, but this is not significant.
  -        if ( currentArticle == -1 && firstArticle > 0 )
  -            currentArticle = firstArticle;
  -        return currentArticle;
  -    }
  -
  -    /**
  -     * @see org.apache.james.nntpserver.NNTPGroup#setCurrentArticleNumber(int)
  -     */
  -    public void setCurrentArticleNumber(int articleNumber) {
  -        this.currentArticle = articleNumber;
  -    }
  -
  -    /**
  -     * @see org.apache.james.nntpserver.NNTPGroup#getCurrentArticle()
  -     */
  -    public NNTPArticle getCurrentArticle() {
  -        return getArticle(getCurrentArticleNumber());
  -    }
  -
  -    /**
        * @see org.apache.james.nntpserver.NNTPGroup#getArticle(int)
        */
       public NNTPArticle getArticle(int number) {
  @@ -184,8 +154,9 @@
               (new DateSinceFileFilter(dt.getTime()),
                new InvertedFileFilter(new ExtensionFileFilter(".id"))));
           List list = new ArrayList();
  -        for ( int i = 0 ; i < f.length ; i++ )
  +        for ( int i = 0 ; i < f.length ; i++ ) {
               list.add(new NNTPArticleImpl(this, f[i]));
  +        }
           return list.iterator();
       }
   
  
  
  
  1.5       +7 -0      jakarta-james/src/java/org/apache/james/nntpserver/repository/NNTPRepository.java
  
  Index: NNTPRepository.java
  ===================================================================
  RCS file: /home/cvs/jakarta-james/src/java/org/apache/james/nntpserver/repository/NNTPRepository.java,v
  retrieving revision 1.4
  retrieving revision 1.5
  diff -u -r1.4 -r1.5
  --- NNTPRepository.java	2 Oct 2002 09:53:38 -0000	1.4
  +++ NNTPRepository.java	26 Oct 2002 04:15:29 -0000	1.5
  @@ -75,4 +75,11 @@
        * @return whether this repository is read only
        */
       boolean isReadOnly();
  +
  +    /**
  +     * Returns the ordered array of header names (including the trailing colon on each)
  +     * returned in overview format for articles stored in this repository.
  +     */
  +    String[] getOverviewFormat();
  +
   }
  
  
  
  1.11      +45 -1     jakarta-james/src/java/org/apache/james/nntpserver/repository/NNTPRepositoryImpl.java
  
  Index: NNTPRepositoryImpl.java
  ===================================================================
  RCS file: /home/cvs/jakarta-james/src/java/org/apache/james/nntpserver/repository/NNTPRepositoryImpl.java,v
  retrieving revision 1.10
  retrieving revision 1.11
  diff -u -r1.10 -r1.11
  --- NNTPRepositoryImpl.java	4 Oct 2002 08:17:49 -0000	1.10
  +++ NNTPRepositoryImpl.java	26 Oct 2002 04:15:29 -0000	1.11
  @@ -102,6 +102,19 @@
       private String articleIDDomainSuffix = null;
   
       /**
  +     * The ordered list of fields returned in the overview format for
  +     * articles stored in this repository.
  +     */
  +    private String[] overviewFormat = { "Subject:",
  +                                        "From:",
  +                                        "Date:",
  +                                        "Message-ID:",
  +                                        "References:",
  +                                        "Bytes:",
  +                                        "Lines:"
  +                                      };
  +
  +    /**
        * This is a mapping of group names to NNTP group objects.
        *
        * TODO: This needs to be addressed so it scales better
  @@ -173,16 +186,41 @@
   
           if ( rootPath.exists() == false ) {
               rootPath.mkdirs();
  +        } else if (!(rootPath.isDirectory())) {
  +            StringBuffer errorBuffer =
  +                new StringBuffer(128)
  +                    .append("NNTP repository root directory is improperly configured.  The specified path ")
  +                    .append(rootPathString)
  +                    .append(" is not a directory.");
  +            throw new ConfigurationException(errorBuffer.toString());
           }
  +
           for ( int i = 0 ; i < addGroups.length ; i++ ) {
               File groupFile = new File(rootPath,addGroups[i]);
               if ( groupFile.exists() == false ) {
                   groupFile.mkdirs();
  +            } else if (!(groupFile.isDirectory())) {
  +                StringBuffer errorBuffer =
  +                    new StringBuffer(128)
  +                        .append("A file exists in the NNTP root directory with the same name as a newsgroup.  File ")
  +                        .append(addGroups[i])
  +                        .append("in directory ")
  +                        .append(rootPathString)
  +                        .append(" is not a directory.");
  +                throw new ConfigurationException(errorBuffer.toString());
               }
           }
           if ( tempPath.exists() == false ) {
               tempPath.mkdirs();
  +        } else if (!(tempPath.isDirectory())) {
  +            StringBuffer errorBuffer =
  +                new StringBuffer(128)
  +                    .append("NNTP repository temp directory is improperly configured.  The specified path ")
  +                    .append(tempPathString)
  +                    .append(" is not a directory.");
  +            throw new ConfigurationException(errorBuffer.toString());
           }
  +
           getLogger().debug("repository initialization done");
       }
   
  @@ -329,6 +367,13 @@
       }
   
       /**
  +     * @see org.apache.james.nntpserver.repository.NNTPRepository#getOverviewFormat()
  +     */
  +    public String[] getOverviewFormat() {
  +        return overviewFormat;
  +    }
  +
  +    /**
        * Creates an instance of the spooler class.
        *
        * TODO: This method doesn't properly implement the Avalon lifecycle.
  @@ -373,5 +418,4 @@
               throw new ConfigurationException("Spooler initialization failed",ex);
           }
       }
  -
   }
  
  
  
  1.11      +37 -29    jakarta-james/src/java/org/apache/james/nntpserver/repository/NNTPSpooler.java
  
  Index: NNTPSpooler.java
  ===================================================================
  RCS file: /home/cvs/jakarta-james/src/java/org/apache/james/nntpserver/repository/NNTPSpooler.java,v
  retrieving revision 1.10
  retrieving revision 1.11
  diff -u -r1.10 -r1.11
  --- NNTPSpooler.java	12 Oct 2002 19:23:19 -0000	1.10
  +++ NNTPSpooler.java	26 Oct 2002 04:15:29 -0000	1.11
  @@ -72,7 +72,7 @@
        */
       public void configure( Configuration configuration ) throws ConfigurationException {
           int threadCount = configuration.getChild("threadCount").getValueAsInteger(1);
  -        threadIdleTime = configuration.getChild("threadIdleTime").getValueAsInteger(1000);
  +        threadIdleTime = configuration.getChild("threadIdleTime").getValueAsInteger(60 * 1000);
           spoolPathString = configuration.getChild("spoolPath").getValue();
           worker = new SpoolerRunnable[threadCount];
       }
  @@ -202,27 +202,33 @@
            */
           public void run() {
               getLogger().debug("in spool thread");
  -            while ( Thread.currentThread().isInterrupted() == false ) {
  -                String[] list = spoolPath.list();
  -                for ( int i = 0 ; i < list.length ; i++ ) {
  -                    getLogger().debug("Files to process: "+list.length);
  -                    if ( lock.lock(list[i]) ) {
  -                        File f = new File(spoolPath,list[i]).getAbsoluteFile();
  -                        getLogger().debug("Processing file: "+f.getAbsolutePath());
  -                        try {
  -                            process(f);
  -                        } catch(Exception ex) {
  -                            getLogger().debug("Exception occured while processing file: "+
  -                                              f.getAbsolutePath(),ex);
  -                        } finally {
  -                            lock.unlock(list[i]);
  +            try {
  +                while ( Thread.currentThread().interrupted() == false ) {
  +                    String[] list = spoolPath.list();
  +                    for ( int i = 0 ; i < list.length ; i++ ) {
  +                        getLogger().debug("Files to process: "+list.length);
  +                        if ( lock.lock(list[i]) ) {
  +                            File f = new File(spoolPath,list[i]).getAbsoluteFile();
  +                            getLogger().debug("Processing file: "+f.getAbsolutePath());
  +                            try {
  +                                process(f);
  +                            } catch(Exception ex) {
  +                                getLogger().debug("Exception occured while processing file: "+
  +                                                  f.getAbsolutePath(),ex);
  +                            } finally {
  +                                lock.unlock(list[i]);
  +                            }
                           }
                       }
  +                    // this is good for other non idle threads
  +                    try {
  +                        Thread.currentThread().sleep(threadIdleTime);
  +                    } catch(InterruptedException ex) {
  +                        // Ignore and continue
  +                    }
                   }
  -                // this is good for other non idle threads
  -                try {
  -                    Thread.currentThread().sleep(threadIdleTime);
  -                } catch(InterruptedException ex) {  }
  +            } finally {
  +                Thread.currentThread().interrupted();
               }
           }
   
  @@ -266,17 +272,19 @@
   
               String[] headers = msg.getHeader("Newsgroups");
               Properties prop = new Properties();
  -            for ( int i = 0 ; i < headers.length ; i++ ) {
  -                getLogger().debug("Copying message to group: "+headers[i]);
  -                NNTPGroup group = repo.getGroup(headers[i]);
  -                if ( group == null ) {
  -                    getLogger().error("Couldn't add article with article ID " + articleID + " to group " + headers[i] + " - group not found.");
  -                    continue;
  -                }
  +            if (headers != null) {
  +                for ( int i = 0 ; i < headers.length ; i++ ) {
  +                    getLogger().debug("Copying message to group: "+headers[i]);
  +                    NNTPGroup group = repo.getGroup(headers[i]);
  +                    if ( group == null ) {
  +                        getLogger().error("Couldn't add article with article ID " + articleID + " to group " + headers[i] + " - group not found.");
  +                        continue;
  +                    }
   
  -                FileInputStream newsStream = new FileInputStream(spoolFile);
  -                NNTPArticle article = group.addArticle(newsStream);
  -                prop.setProperty(group.getName(),article.getArticleNumber() + "");
  +                    FileInputStream newsStream = new FileInputStream(spoolFile);
  +                    NNTPArticle article = group.addArticle(newsStream);
  +                    prop.setProperty(group.getName(),article.getArticleNumber() + "");
  +                }
               }
               articleIDRepo.addArticle(articleID,prop);
               boolean delSuccess = spoolFile.delete();
  
  
  
  1.13      +8 -8      jakarta-james/src/conf/james-assembly.xml
  
  Index: james-assembly.xml
  ===================================================================
  RCS file: /home/cvs/jakarta-james/src/conf/james-assembly.xml,v
  retrieving revision 1.12
  retrieving revision 1.13
  diff -u -r1.12 -r1.13
  --- james-assembly.xml	7 Oct 2002 07:16:46 -0000	1.12
  +++ james-assembly.xml	26 Oct 2002 04:15:29 -0000	1.13
  @@ -54,9 +54,9 @@
                role="org.apache.avalon.cornerstone.services.sockets.SocketManager"/>
       <provide name="connections"
                role="org.apache.avalon.cornerstone.services.connection.ConnectionManager"/>
  -    <provide name="scheduler"
  -             role="org.apache.avalon.cornerstone.services.scheduler.TimeScheduler"/>
       <provide name="James" role="org.apache.james.services.MailServer"/>
  +    <provide name="thread-manager"
  +             role="org.apache.avalon.cornerstone.services.threads.ThreadManager" />
     </block>
   
     <!-- POP3 Server -->
  @@ -67,9 +67,9 @@
                role="org.apache.avalon.cornerstone.services.sockets.SocketManager"/>
       <provide name="connections"
                role="org.apache.avalon.cornerstone.services.connection.ConnectionManager"/>
  -    <provide name="scheduler"
  -             role="org.apache.avalon.cornerstone.services.scheduler.TimeScheduler"/>
       <provide name="James" role="org.apache.james.services.MailServer"/>
  +    <provide name="thread-manager"
  +             role="org.apache.avalon.cornerstone.services.threads.ThreadManager" />
     </block>
   
     <!-- SMTP Server -->
  @@ -81,9 +81,9 @@
                role="org.apache.avalon.cornerstone.services.sockets.SocketManager"/>
       <provide name="connections"
                role="org.apache.avalon.cornerstone.services.connection.ConnectionManager"/>
  -    <provide name="scheduler"
  -             role="org.apache.avalon.cornerstone.services.scheduler.TimeScheduler"/>
       <provide name="James" role="org.apache.james.services.MailServer"/>
  +    <provide name="thread-manager"
  +             role="org.apache.avalon.cornerstone.services.threads.ThreadManager" />
     </block>
   
     <!-- NNTP Server -->
  @@ -93,10 +93,10 @@
                role="org.apache.avalon.cornerstone.services.sockets.SocketManager"/>
       <provide name="connections"
                role="org.apache.avalon.cornerstone.services.connection.ConnectionManager"/>
  -    <provide name="scheduler"
  -             role="org.apache.avalon.cornerstone.services.scheduler.TimeScheduler"/>
       <provide name="nntp-repository"
                role="org.apache.james.nntpserver.repository.NNTPRepository"/>
  +    <provide name="thread-manager"
  +             role="org.apache.avalon.cornerstone.services.threads.ThreadManager" />
     </block>
   
     <!-- NNTP Repository -->
  
  
  
  1.11      +2 -2      jakarta-james/src/conf/james-server.xml
  
  Index: james-server.xml
  ===================================================================
  RCS file: /home/cvs/jakarta-james/src/conf/james-server.xml,v
  retrieving revision 1.10
  retrieving revision 1.11
  diff -u -r1.10 -r1.11
  --- james-server.xml	3 Oct 2002 00:33:50 -0000	1.10
  +++ james-server.xml	26 Oct 2002 04:15:29 -0000	1.11
  @@ -76,7 +76,7 @@
         </category>
         <category name="fetchpop" log-level="INFO">
           <log-target id-ref="fetchpop-target"/>
  -      </category>      
  +      </category>
       </categories>
   
       <!-- Logger targets -->
  @@ -177,7 +177,7 @@
           <filename>${app.home}/logs/fetchpop.log</filename>
           <format>%{time:dd/MM/yy HH:mm:ss} %5.5{priority} %{category}: %{message}\n%{throwable}</format>
           <append>true</append>
  -      </file>      
  +      </file>
       </targets>
     </logs>
   
  
  
  
  1.15      +174 -80   jakarta-james/src/java/org/apache/james/pop3server/POP3Handler.java
  
  Index: POP3Handler.java
  ===================================================================
  RCS file: /home/cvs/jakarta-james/src/java/org/apache/james/pop3server/POP3Handler.java,v
  retrieving revision 1.14
  retrieving revision 1.15
  diff -u -r1.14 -r1.15
  --- POP3Handler.java	17 Oct 2002 22:04:00 -0000	1.14
  +++ POP3Handler.java	26 Oct 2002 04:15:30 -0000	1.15
  @@ -8,17 +8,11 @@
   package org.apache.james.pop3server;
   
   import org.apache.avalon.cornerstone.services.connection.ConnectionHandler;
  -import org.apache.avalon.cornerstone.services.scheduler.PeriodicTimeTrigger;
  -import org.apache.avalon.cornerstone.services.scheduler.Target;
  -import org.apache.avalon.cornerstone.services.scheduler.TimeScheduler;
   import org.apache.avalon.excalibur.collections.ListUtils;
  -import org.apache.avalon.framework.component.ComponentException;
  -import org.apache.avalon.framework.component.ComponentManager;
  -import org.apache.avalon.framework.component.Composable;
  -import org.apache.avalon.framework.configuration.Configurable;
  -import org.apache.avalon.framework.configuration.Configuration;
  -import org.apache.avalon.framework.configuration.ConfigurationException;
  -import org.apache.james.BaseConnectionHandler;
  +import org.apache.avalon.excalibur.pool.Poolable;
  +import org.apache.avalon.framework.activity.Disposable;
  +import org.apache.avalon.framework.logger.AbstractLogEnabled;
  +
   import org.apache.james.Constants;
   import org.apache.james.core.MailImpl;
   import org.apache.james.services.MailRepository;
  @@ -27,7 +21,9 @@
   import org.apache.james.services.UsersStore;
   import org.apache.james.util.ExtraDotOutputStream;
   import org.apache.james.util.InternetPrintWriter;
  -import org.apache.james.util.SchedulerNotifyOutputStream;
  +import org.apache.james.util.watchdog.BytesWrittenResetOutputStream;
  +import org.apache.james.util.watchdog.Watchdog;
  +import org.apache.james.util.watchdog.WatchdogTarget;
   import org.apache.mailet.Mail;
   
   import javax.mail.MessagingException;
  @@ -39,11 +35,11 @@
    * The handler class for POP3 connections.
    *
    * @author Federico Barbieri <sc...@systemy.it>
  - * @version 0.9
  + * @author Peter M. Goldstein <fa...@alum.mit.edu>
    */
   public class POP3Handler
  -    extends BaseConnectionHandler
  -    implements ConnectionHandler, Composable, Configurable, Target {
  +    extends AbstractLogEnabled
  +    implements ConnectionHandler, Poolable {
   
       // POP3 Server identification string used in POP3 headers
       private static final String softwaretype        = "JAMES POP3 Server "
  @@ -78,17 +74,9 @@
                                                             // deleted from the inbox.
   
       /**
  -     * The internal mail server service
  +     * The per-service configuration data that applies to all handlers
        */
  -    private MailServer mailServer;
  -
  -    /**
  -     * The user repository for this server - used to authenticate users.
  -     */
  -    private UsersRepository users;
  -
  -    private TimeScheduler scheduler;    // The scheduler used to handle timeouts for the
  -                                        // POP3 interaction
  +    private POP3HandlerConfigurationData theConfigData;
   
       /**
        * The mail server's copy of the user's inbox
  @@ -111,49 +99,84 @@
        */
       private PrintWriter out;
   
  -    private OutputStream outs;     // The socket's output stream
  +    /**
  +     * The socket's output stream
  +     */
  +    private OutputStream outs;
   
  -    private int state;             // The current transaction state of the handler
  +    /**
  +     * The current transaction state of the handler
  +     */
  +    private int state;
   
       /**
        * The user id associated with the POP3 dialogue
        */
       private String user;
   
  -    private Vector userMailbox = new Vector();   // A dynamic list representing the set of
  -                                                 // emails in the user's inbox at any given time
  -                                                 // during the POP3 transaction
  +    /**
  +     * A dynamic list representing the set of
  +     * emails in the user's inbox at any given time
  +     * during the POP3 transaction.
  +     */
  +    private Vector userMailbox = new Vector();
   
       private Vector backupUserMailbox;            // A snapshot list representing the set of 
                                                    // emails in the user's inbox at the beginning
                                                    // of the transaction
   
  -    private int lengthReset = 20000;         // The number of bytes to read before resetting
  -                                             // the connection timeout timer.  Defaults to
  -                                             // 20 seconds.
  +    /**
  +     * The watchdog being used by this handler to deal with idle timeouts.
  +     */
  +    private Watchdog theWatchdog;
  +
  +    /**
  +     * The watchdog target that idles out this handler.
  +     */
  +    private WatchdogTarget theWatchdogTarget = new POP3WatchdogTarget();
  +
  +    /**
  +     * Set the configuration data for the handler.
  +     *
  +     * @param theData the configuration data
  +     */
  +    void setConfigurationData(POP3HandlerConfigurationData theData) {
  +        theConfigData = theData;
  +    }
   
       /**
  -     * @see org.apache.avalon.framework.component.Composable#compose(ComponentManager)
  +     * Set the Watchdog for use by this handler.
  +     *
  +     * @param theWatchdog the watchdog
        */
  -    public void compose( final ComponentManager componentManager )
  -        throws ComponentException {
  -        mailServer = (MailServer)componentManager.
  -            lookup( "org.apache.james.services.MailServer" );
  -        UsersStore usersStore = (UsersStore)componentManager.
  -            lookup( "org.apache.james.services.UsersStore" );
  -        users = usersStore.getRepository("LocalUsers");
  -        scheduler = (TimeScheduler)componentManager.
  -            lookup( "org.apache.avalon.cornerstone.services.scheduler.TimeScheduler" );
  +    void setWatchdog(Watchdog theWatchdog) {
  +        this.theWatchdog = theWatchdog;
       }
   
       /**
  -     * @see org.apache.avalon.framework.configuration.Configurable#configure(Configuration)
  +     * Gets the Watchdog Target that should be used by Watchdogs managing
  +     * this connection.
  +     *
  +     * @return the WatchdogTarget
        */
  -    public void configure(Configuration configuration)
  -            throws ConfigurationException {
  -        super.configure(configuration);
  +    WatchdogTarget getWatchdogTarget() {
  +        return theWatchdogTarget;
  +    }
   
  -        lengthReset = configuration.getChild("lengthReset").getValueAsInteger(20000);
  +    /**
  +     * Idle out this connection
  +     */
  +    void idleClose() {
  +        if (getLogger() != null) {
  +            getLogger().error("POP3 Connection has idled out.");
  +        }
  +        try {
  +            if (socket != null) {
  +                socket.close();
  +            }
  +        } catch (Exception e) {
  +            // ignored
  +        }
       }
   
       /**
  @@ -167,9 +190,7 @@
   
           try {
               this.socket = connection;
  -            in = new BufferedReader(new InputStreamReader(socket.getInputStream()));
  -            outs = socket.getOutputStream();
  -            out = new InternetPrintWriter(outs, true);
  +            in = new BufferedReader(new InputStreamReader(socket.getInputStream(), "ASCII"), 512);
               remoteIP = socket.getInetAddress().getHostAddress ();
               remoteHost = socket.getInetAddress().getHostName ();
           } catch (Exception e) {
  @@ -198,22 +219,25 @@
           }
   
           try {
  -            final PeriodicTimeTrigger trigger = new PeriodicTimeTrigger( timeout, -1 );
  -            scheduler.addTrigger( this.toString(), trigger, this );
  +            outs = socket.getOutputStream();
  +            out = new InternetPrintWriter(outs, true);
               state = AUTHENTICATION_READY;
               user = "unknown";
               StringBuffer responseBuffer =
                   new StringBuffer(256)
                           .append(OK_RESPONSE)
                           .append(" ")
  -                        .append(this.helloName)
  +                        .append(theConfigData.getHelloName())
                           .append(" POP3 server (")
                           .append(this.softwaretype)
                           .append(") ready ");
               out.println(responseBuffer.toString());
  +
  +            theWatchdog.start();
               while (parseCommand(in.readLine())) {
  -                scheduler.resetTrigger(this.toString());
  +                theWatchdog.reset();
               }
  +            theWatchdog.stop();
               if (getLogger().isInfoEnabled()) {
                   StringBuffer logBuffer =
                       new StringBuffer(128)
  @@ -238,31 +262,81 @@
                           .append(") : ")
                           .append(e.getMessage());
               getLogger().error(exceptionBuffer.toString(), e );
  -            try {
  -                socket.close();
  -            } catch (IOException ioe) {  }
  -
  -            // release from scheduler.
  -            try {
  -                scheduler.removeTrigger(this.toString());
  -            } catch(Throwable t) { }
  +        } finally {
  +            resetHandler();
           }
       }
   
       /**
  -     * Callback method called when the the PeriodicTimeTrigger in 
  -     * handleConnection is triggered.  In this case the trigger is
  -     * being used as a timeout, so the method simply closes the connection.
  -     *
  -     * @param triggerName the name of the trigger
  +     * Resets the handler data to a basic state.
        */
  -    public void targetTriggered( final String triggerName ) {
  -        getLogger().error("Connection timeout on socket");
  +    private void resetHandler() {
  +
  +        if (theWatchdog != null) {
  +            if (theWatchdog instanceof Disposable) {
  +                ((Disposable)theWatchdog).dispose();
  +            }
  +            theWatchdog = null;
  +        }
  +
  +        // Close and clear streams, sockets
  +
           try {
  -            out.println("Connection timeout. Closing connection");
  -            socket.close();
  -        } catch (IOException e) {
  +            if (socket != null) {
  +                socket.close();
  +                socket = null;
  +            }
  +        } catch (IOException ioe) {
  +            // Ignoring exception on close
  +        } finally {
  +            socket = null;
           }
  +
  +        try {
  +            if (in != null) {
  +                in.close();
  +            }
  +        } catch (Exception e) {
  +            // Ignored
  +        } finally {
  +            in = null;
  +        }
  +
  +        try {
  +            if (out != null) {
  +                out.close();
  +            }
  +        } catch (Exception e) {
  +            // Ignored
  +        } finally {
  +            out = null;
  +        }
  +
  +        try {
  +           if (outs != null) {
  +               outs.close();
  +            }
  +        } catch (Exception e) {
  +            // Ignored
  +        } finally {
  +            outs = null;
  +        }
  +
  +        // Clear user data
  +        user = null;
  +        userInbox = null;
  +        if (userMailbox != null) {
  +            userMailbox.clear();
  +            userMailbox = null;
  +        }
  +
  +        if (backupUserMailbox != null) {
  +            backupUserMailbox.clear();
  +            backupUserMailbox = null;
  +        }
  +
  +        // Clear config data
  +        theConfigData = null;
       }
   
       /**
  @@ -396,7 +470,7 @@
           String responseString = null;
           if (state == AUTHENTICATION_USERSET && argument != null) {
               String passArg = argument;
  -            if (users.test(user, passArg)) {
  +            if (theConfigData.getUsersRepository().test(user, passArg)) {
                   StringBuffer responseBuffer =
                       new StringBuffer(64)
                               .append(OK_RESPONSE)
  @@ -405,7 +479,7 @@
                   responseString = responseBuffer.toString();
                   state = TRANSACTION;
                   out.println(responseString);
  -                userInbox = mailServer.getUserInbox(user);
  +                userInbox = theConfigData.getMailServer().getUserInbox(user);
                   stat();
               } else {
                   responseString = ERR_RESPONSE + " Authentication failed.";
  @@ -775,9 +849,10 @@
                       responseString = OK_RESPONSE + " Message follows";
                       out.println(responseString);
                       OutputStream nouts =
  -                            new ExtraDotOutputStream(
  -                            new SchedulerNotifyOutputStream(outs, scheduler,
  -                            this.toString(), lengthReset));
  +                            new ExtraDotOutputStream(outs);
  +                    nouts = new BytesWrittenResetOutputStream(nouts,
  +                                                              theWatchdog,
  +                                                              theConfigData.getResetLength());
                       mc.writeMessageTo(nouts);
                       out.println();
                       out.println(".");
  @@ -853,9 +928,10 @@
                       }
                       out.println("");
                       OutputStream nouts =
  -                            new ExtraDotOutputStream(
  -                            new SchedulerNotifyOutputStream(outs, scheduler,
  -                            this.toString(), lengthReset));
  +                            new ExtraDotOutputStream(outs);
  +                    nouts = new BytesWrittenResetOutputStream(nouts,
  +                                                              theWatchdog,
  +                                                              theConfigData.getResetLength());
                       mc.writeContentTo(nouts, lines);
                       out.println(".");
                   } else {
  @@ -950,5 +1026,23 @@
               getLogger().debug("Sent: " + responseString);
           }
       }
  +
  +    /**
  +     * A private inner class which serves as an adaptor
  +     * between the WatchdogTarget interface and this
  +     * handler class.
  +     */
  +    private class POP3WatchdogTarget
  +        implements WatchdogTarget {
  +
  +        /**
  +         * @see org.apache.james.util.watchdog.WatchdogTarget#execute()
  +         */
  +        public void execute() {
  +            POP3Handler.this.idleClose();
  +        }
  +
  +    }
  +
   }
   
  
  
  
  1.12      +199 -51   jakarta-james/src/java/org/apache/james/pop3server/POP3Server.java
  
  Index: POP3Server.java
  ===================================================================
  RCS file: /home/cvs/jakarta-james/src/java/org/apache/james/pop3server/POP3Server.java,v
  retrieving revision 1.11
  retrieving revision 1.12
  diff -u -r1.11 -r1.12
  --- POP3Server.java	2 Oct 2002 06:12:02 -0000	1.11
  +++ POP3Server.java	26 Oct 2002 04:15:30 -0000	1.12
  @@ -6,12 +6,32 @@
    * the LICENSE file.
    */
   package org.apache.james.pop3server;
  -import org.apache.avalon.cornerstone.services.connection.AbstractService;
  -import org.apache.avalon.cornerstone.services.connection.ConnectionHandlerFactory;
  -import org.apache.avalon.cornerstone.services.connection.DefaultHandlerFactory;
  +
  +import org.apache.avalon.cornerstone.services.connection.ConnectionHandler;
  +import org.apache.avalon.excalibur.pool.DefaultPool;
  +import org.apache.avalon.excalibur.pool.HardResourceLimitingPool;
  +import org.apache.avalon.excalibur.pool.ObjectFactory;
  +import org.apache.avalon.excalibur.pool.Pool;
  +import org.apache.avalon.excalibur.pool.Poolable;
  +import org.apache.avalon.framework.activity.Disposable;
  +import org.apache.avalon.framework.activity.Initializable;
   import org.apache.avalon.framework.configuration.Configuration;
   import org.apache.avalon.framework.configuration.ConfigurationException;
   import org.apache.avalon.framework.component.Component;
  +import org.apache.avalon.framework.component.ComponentException;
  +import org.apache.avalon.framework.component.ComponentManager;
  +import org.apache.avalon.framework.component.Composable;
  +import org.apache.avalon.framework.logger.LogEnabled;
  +
  +import org.apache.james.core.AbstractJamesService;
  +import org.apache.james.services.MailServer;
  +import org.apache.james.services.UsersRepository;
  +import org.apache.james.services.UsersStore;
  +import org.apache.james.util.watchdog.ThreadPerWatchdogFactory;
  +import org.apache.james.util.watchdog.Watchdog;
  +import org.apache.james.util.watchdog.WatchdogFactory;
  +import org.apache.james.util.watchdog.WatchdogTarget;
  +
   import java.net.InetAddress;
   import java.net.UnknownHostException;
   /**
  @@ -22,43 +42,75 @@
    * @version 1.0.0, 24/04/1999
    * @author  Federico Barbieri <sc...@pop.systemy.it>
    * @author  <a href="mailto:danny@apache.org">Danny Angus</a>
  + * @author Peter M. Goldstein <fa...@alum.mit.edu>
    */
  -public class POP3Server extends AbstractService implements Component {
  +public class POP3Server extends AbstractJamesService implements Component {
  +
  +    /**
  +     * The internal mail server service
  +     */
  +    private MailServer mailServer;
  +
  +    /**
  +     * The user repository for this server - used to authenticate users.
  +     */
  +    private UsersRepository users;
   
       /**
  -     * Whether this service is enabled
  +     * The number of bytes to read before resetting
  +     * the connection timeout timer.  Defaults to
  +     * 20 KB.
        */
  -    private volatile boolean enabled = true;
  +    private int lengthReset = 20 * 1024;
   
       /**
  -     * Creates a subclass specific handler factory for use by the superclass.
  -     *
  -     * @return a ConnectionHandlerFactory that produces POP3Handlers
  +     * The pool used to provide POP3 Handler objects
        */
  -    protected ConnectionHandlerFactory createFactory() {
  -        return new DefaultHandlerFactory(POP3Handler.class);
  +    private Pool theHandlerPool = null;
  +
  +    /**
  +     * The factory used to provide POP3 Handler objects
  +     */
  +    private ObjectFactory theHandlerFactory = new POP3HandlerFactory();
  +
  +    /**
  +     * The factory used to generate Watchdog objects
  +     */
  +    private WatchdogFactory theWatchdogFactory;
  +
  +    /**
  +     * The configuration data to be passed to the handler
  +     */
  +    private POP3HandlerConfigurationData theConfigData
  +        = new POP3HandlerConfigurationDataImpl();
  +
  +    /**
  +     * @see org.apache.avalon.framework.component.Composable#compose(ComponentManager)
  +     */
  +    public void compose( final ComponentManager componentManager )
  +        throws ComponentException {
  +        super.compose(componentManager);
  +        mailServer = (MailServer)componentManager.
  +            lookup( "org.apache.james.services.MailServer" );
  +        UsersStore usersStore = (UsersStore)componentManager.
  +            lookup( "org.apache.james.services.UsersStore" );
  +        users = usersStore.getRepository("LocalUsers");
  +        if (users == null) {
  +            throw new ComponentException("The user repository could not be found.");
  +        }
       }
   
       /**
        * @see org.apache.avalon.framework.configuration.Configurable#configure(Configuration)
        */
       public void configure(final Configuration configuration) throws ConfigurationException {
  -        enabled = configuration.getAttributeAsBoolean("enabled", true);
  -        if (enabled) {
  -            m_port = configuration.getChild("port").getValueAsInteger(25);
  -            try {
  -                final String bindAddress = configuration.getChild("bind").getValue(null);
  -                if (null != bindAddress) {
  -                    m_bindTo = InetAddress.getByName(bindAddress);
  -                }
  -            } catch (final UnknownHostException unhe) {
  -                throw new ConfigurationException("Malformed bind parameter", unhe);
  -            }
  -            final boolean useTLS = configuration.getChild("useTLS").getValueAsBoolean(false);
  -            if (useTLS) {
  -                m_serverSocketType = "ssl";
  +        super.configure(configuration);
  +        if (isEnabled()) {
  +            Configuration handlerConfiguration = configuration.getChild("handler");
  +            lengthReset = handlerConfiguration.getChild("lengthReset").getValueAsInteger(lengthReset);
  +            if (getLogger().isInfoEnabled()) {
  +                getLogger().info("The idle timeout will be reset every " + lengthReset + " bytes.");
               }
  -            super.configure(configuration.getChild("handler"));
           }
       }
   
  @@ -66,37 +118,133 @@
        * @see org.apache.avalon.framework.activity.Initializable#initialize()
        */
       public void initialize() throws Exception {
  -        if (enabled) {
  -            getLogger().info("POP3Server init...");
  -            StringBuffer logBuffer =
  -                new StringBuffer(128)
  -                    .append("POP3Listener using ")
  -                    .append(m_serverSocketType)
  -                    .append(" on port ")
  -                    .append(m_port)
  -                    .append(" at ")
  -                    .append(m_bindTo);
  -            getLogger().info(logBuffer.toString());
  -            super.initialize();
  -            getLogger().info("POP3Server ...init end");
  -            System.out.println("POP3 Server Started " + m_connectionName);
  +        super.initialize();
  +        if (!isEnabled()) {
  +            return;
  +        }
  +
  +        if (connectionLimit != null) {
  +            theHandlerPool = new HardResourceLimitingPool(theHandlerFactory, 5, connectionLimit.intValue());
  +            getLogger().debug("Using a bounded pool for POP3 handlers with upper limit " + connectionLimit.intValue());
           } else {
  -            getLogger().info("POP3Server Disabled");
  -            System.out.println("POP3 Server Disabled");
  +            // NOTE: The maximum here is not a real maximum.  The handler pool will continue to
  +            //       provide handlers beyond this value.
  +            theHandlerPool = new DefaultPool(theHandlerFactory, null, 5, 30);
  +            getLogger().debug("Using an unbounded pool for POP3 handlers.");
  +        }
  +        if (theHandlerPool instanceof LogEnabled) {
  +            ((LogEnabled)theHandlerPool).enableLogging(getLogger());
  +        }
  +        if (theHandlerPool instanceof Initializable) {
  +            ((Initializable)theHandlerPool).initialize();
  +        }
  +
  +        theWatchdogFactory = new ThreadPerWatchdogFactory(threadPool, timeout);
  +        if (theWatchdogFactory instanceof LogEnabled) {
  +            ((LogEnabled)theWatchdogFactory).enableLogging(getLogger());
  +        }
  +    }
  +
  +    /**
  +     * @see org.apache.james.core.AbstractJamesService#getDefaultPort()
  +     */
  +     protected int getDefaultPort() {
  +        return 110;
  +     }
  +
  +    /**
  +     * @see org.apache.james.core.AbstractJamesService#getServiceType()
  +     */
  +    public String getServiceType() {
  +        return "POP3 Service";
  +    }
  +
  +    /**
  +     * @see org.apache.avalon.cornerstone.services.connection.AbstractHandlerFactory#newHandler()
  +     */
  +    protected ConnectionHandler newHandler()
  +            throws Exception {
  +        POP3Handler theHandler = (POP3Handler)theHandlerPool.get();
  +
  +        Watchdog theWatchdog = theWatchdogFactory.getWatchdog(theHandler.getWatchdogTarget());
  +
  +        theHandler.setConfigurationData(theConfigData);
  +
  +        theHandler.setWatchdog(theWatchdog);
  +
  +        return theHandler;
  +    }
  +
  +    /**
  +     * @see org.apache.avalon.cornerstone.services.connection.ConnectionHandlerFactory#releaseConnectionHandler(ConnectionHandler)
  +     */
  +    public void releaseConnectionHandler( ConnectionHandler connectionHandler ) {
  +        if (!(connectionHandler instanceof POP3Handler)) {
  +            throw new IllegalArgumentException("Attempted to return non-POP3Handler to pool.");
           }
  +        theHandlerPool.put((Poolable)connectionHandler);
       }
   
       /**
  -     * @see org.apache.avalon.framework.activity.Disposable#dispose()
  +     * The factory for producing handlers.
        */
  -    public void dispose() {
  -        if (enabled) {
  -            getLogger().info("POP3Server dispose...");
  -            getLogger().info("POP3Server dispose..." + m_connectionName);
  -            super.dispose();
  -            // This is needed to make sure sockets are promptly closed on Windows 2000
  -            System.gc();
  -            getLogger().info("POP3Server ...dispose end");
  +    private static class POP3HandlerFactory
  +        implements ObjectFactory {
  +
  +        /**
  +         * @see org.apache.avalon.excalibur.pool.ObjectFactory#newInstance()
  +         */
  +        public Object newInstance() throws Exception {
  +            return new POP3Handler();
  +        }
  +
  +        /**
  +         * @see org.apache.avalon.excalibur.pool.ObjectFactory#getCreatedClass()
  +         */
  +        public Class getCreatedClass() {
  +            return POP3Handler.class;
  +        }
  +
  +        /**
  +         * @see org.apache.avalon.excalibur.pool.ObjectFactory#decommision(Object)
  +         */
  +        public void decommission( Object object ) throws Exception {
  +            return;
  +        }
  +    }
  +
  +    /**
  +     * A class to provide POP3 handler configuration to the handlers
  +     */
  +    private class POP3HandlerConfigurationDataImpl
  +        implements POP3HandlerConfigurationData {
  +
  +        /**
  +         * @see org.apache.james.pop3server.POP3HandlerConfigurationData#getHelloName()
  +         */
  +        public String getHelloName() {
  +            return POP3Server.this.helloName;
  +        }
  +
  +        /**
  +         * @see org.apache.james.pop3server.POP3HandlerConfigurationData#getResetLength()
  +         */
  +        public int getResetLength() {
  +            return POP3Server.this.lengthReset;
  +        }
  +
  +        /**
  +         * @see org.apache.james.pop3server.POP3HandlerConfigurationData#getMailServer()
  +         */
  +        public MailServer getMailServer() {
  +            return POP3Server.this.mailServer;
  +        }
  +
  +        /**
  +         * @see org.apache.james.pop3server.POP3HandlerConfigurationData#getUsersRepository()
  +         */
  +        public UsersRepository getUsersRepository() {
  +            return POP3Server.this.users;
           }
       }
   }
  
  
  
  1.3       +3 -3      jakarta-james/src/java/org/apache/james/pop3server/POP3Server.xinfo
  
  Index: POP3Server.xinfo
  ===================================================================
  RCS file: /home/cvs/jakarta-james/src/java/org/apache/james/pop3server/POP3Server.xinfo,v
  retrieving revision 1.2
  retrieving revision 1.3
  diff -u -r1.2 -r1.3
  --- POP3Server.xinfo	25 Sep 2001 04:51:19 -0000	1.2
  +++ POP3Server.xinfo	26 Oct 2002 04:15:30 -0000	1.3
  @@ -27,10 +27,10 @@
         <service name="org.apache.avalon.cornerstone.services.sockets.SocketManager" version="1.0"/>
       </dependency>
       <dependency>
  -      <service name="org.apache.avalon.cornerstone.services.scheduler.TimeScheduler" version="1.0"/>
  -    </dependency> 
  -    <dependency>
         <service name="org.apache.james.services.MailServer" version="1.0"/>
  +    </dependency>
  +    <dependency>
  +      <service name="org.apache.avalon.cornerstone.services.threads.ThreadManager" version="1.0"/>
       </dependency>
     </dependencies>
   
  
  
  
  1.1                  jakarta-james/src/java/org/apache/james/pop3server/POP3HandlerConfigurationData.java
  
  Index: POP3HandlerConfigurationData.java
  ===================================================================
  /*
   * Copyright (C) The Apache Software Foundation. All rights reserved.
   *
   * This software is published under the terms of the Apache Software License
   * version 1.1, a copy of which has been included with this distribution in
   * the LICENSE file.
   */
  package org.apache.james.pop3server;
  
  import org.apache.james.services.MailServer;
  import org.apache.james.services.UsersRepository;
  
  /**
   * Provides a number of server-wide constant values to the
   * POP3Handlers
   *
   * @author Peter M. Goldstein <fa...@alum.mit.edu>
   */
  public interface POP3HandlerConfigurationData {
  
      /**
       * Returns the service wide hello name
       *
       * @return the hello name
       */
      String getHelloName();
  
      /**
       * Returns the service wide reset length in bytes.
       *
       * @return the reset length
       */
      int getResetLength();
  
      /**
       * Returns the MailServer interface for this service.
       *
       * @return the MailServer interface for this service
       */
      MailServer getMailServer();
  
      /**
       * Returns the UsersRepository for this service.
       *
       * @return the local users repository
       */
      UsersRepository getUsersRepository();
  
  }
  
  
  
  1.11      +214 -46   jakarta-james/src/java/org/apache/james/remotemanager/RemoteManager.java
  
  Index: RemoteManager.java
  ===================================================================
  RCS file: /home/cvs/jakarta-james/src/java/org/apache/james/remotemanager/RemoteManager.java,v
  retrieving revision 1.10
  retrieving revision 1.11
  diff -u -r1.10 -r1.11
  --- RemoteManager.java	2 Oct 2002 06:12:02 -0000	1.10
  +++ RemoteManager.java	26 Oct 2002 04:15:30 -0000	1.11
  @@ -7,15 +7,32 @@
    */
   package org.apache.james.remotemanager;
   
  -import org.apache.avalon.cornerstone.services.connection.AbstractService;
  -import org.apache.avalon.cornerstone.services.connection.ConnectionHandlerFactory;
  -import org.apache.avalon.cornerstone.services.connection.DefaultHandlerFactory;
  +import org.apache.avalon.cornerstone.services.connection.ConnectionHandler;
  +import org.apache.avalon.excalibur.pool.DefaultPool;
  +import org.apache.avalon.excalibur.pool.HardResourceLimitingPool;
  +import org.apache.avalon.excalibur.pool.ObjectFactory;
  +import org.apache.avalon.excalibur.pool.Pool;
  +import org.apache.avalon.excalibur.pool.Poolable;
  +import org.apache.avalon.framework.activity.Disposable;
  +import org.apache.avalon.framework.activity.Initializable;
   import org.apache.avalon.framework.configuration.Configuration;
   import org.apache.avalon.framework.configuration.ConfigurationException;
   import org.apache.avalon.framework.component.Component;
  +import org.apache.avalon.framework.component.ComponentException;
  +import org.apache.avalon.framework.component.ComponentManager;
  +import org.apache.avalon.framework.component.Composable;
  +import org.apache.avalon.framework.logger.LogEnabled;
  +
  +import org.apache.james.core.AbstractJamesService;
  +import org.apache.james.services.*;
  +import org.apache.james.util.watchdog.ThreadPerWatchdogFactory;
  +import org.apache.james.util.watchdog.Watchdog;
  +import org.apache.james.util.watchdog.WatchdogFactory;
  +import org.apache.james.util.watchdog.WatchdogTarget;
   
   import java.net.InetAddress;
   import java.net.UnknownHostException;
  +import java.util.HashMap;
   
   /**
    * Provides a really rude network interface to administer James.
  @@ -26,12 +43,66 @@
    * @version 1.0.0, 24/04/1999
    * @author  Federico Barbieri <sc...@pop.systemy.it>
    * @author <a href="mailto:donaldp@apache.org">Peter Donald</a>
  + * @author Peter M. Goldstein <fa...@alum.mit.edu>
    */
   public class RemoteManager
  -    extends AbstractService implements Component {
  +    extends AbstractJamesService implements Component {
   
  -    protected ConnectionHandlerFactory createFactory() {
  -        return new DefaultHandlerFactory( RemoteManagerHandler.class );
  +    /**
  +     * A HashMap of (user id, passwords) for James administrators
  +     */
  +    private HashMap adminAccounts = new HashMap();
  +
  +    /**
  +     * The UsersStore that contains all UsersRepositories managed by this RemoteManager
  +     */
  +    private UsersStore usersStore;
  +
  +    /**
  +     * The current UsersRepository being managed/viewed/modified
  +     */
  +    private UsersRepository users;
  +
  +    /**
  +     * The reference to the internal MailServer service
  +     */
  +    private MailServer mailServer;
  +
  +    /**
  +     * The pool used to provide RemoteManager Handler objects
  +     */
  +    private Pool theHandlerPool = null;
  +
  +    /**
  +     * The pool used to provide RemoteManager Handler objects
  +     */
  +    private ObjectFactory theHandlerFactory = new RemoteManagerHandlerFactory();
  +
  +    /**
  +     * The factory used to generate Watchdog objects
  +     */
  +    private WatchdogFactory theWatchdogFactory;
  +
  +    /**
  +     * The configuration data to be passed to the handler
  +     */
  +    private RemoteManagerHandlerConfigurationData theConfigData
  +        = new RemoteManagerHandlerConfigurationDataImpl();
  +
  +    /**
  +     * @see org.apache.avalon.framework.component.Composable#compose(ComponentManager)
  +     */
  +    public void compose( final ComponentManager componentManager )
  +        throws ComponentException {
  +        super.compose(componentManager);
  +        mailServer = (MailServer)componentManager.
  +            lookup( "org.apache.james.services.MailServer" );
  +        usersStore = (UsersStore)componentManager.
  +            lookup( "org.apache.james.services.UsersStore" );
  +        users = usersStore.getRepository("LocalUsers");
  +        if (users == null) {
  +            throw new ComponentException("The user repository could not be found.");
  +        }
       }
   
       /**
  @@ -40,59 +111,156 @@
       public void configure( final Configuration configuration )
           throws ConfigurationException {
   
  -        m_port = configuration.getChild( "port" ).getValueAsInteger( 4555 );
  -
  -        try
  -        {
  -            final String bindAddress = configuration.getChild( "bind" ).getValue( null );
  -            if( null != bindAddress )
  -            {
  -                m_bindTo = InetAddress.getByName( bindAddress );
  +        super.configure(configuration);
  +        if (isEnabled()) {
  +            Configuration handlerConfiguration = configuration.getChild("handler");
  +            Configuration admin = handlerConfiguration.getChild( "administrator_accounts" );
  +            Configuration[] accounts = admin.getChildren( "account" );
  +            for ( int i = 0; i < accounts.length; i++ ) {
  +                adminAccounts.put( accounts[ i ].getAttribute( "login" ),
  +                                   accounts[ i ].getAttribute( "password" ) );
               }
           }
  -        catch( final UnknownHostException unhe )
  -        {
  -            throw new ConfigurationException( "Malformed bind parameter", unhe );
  +    }
  +
  +    /**
  +     * @see org.apache.avalon.framework.activity.Initializable#initialize()
  +     */
  +    public void initialize() throws Exception {
  +        super.initialize();
  +        if (!isEnabled()) {
  +            return;
  +        }
  +
  +        if (connectionLimit != null) {
  +            theHandlerPool = new HardResourceLimitingPool(theHandlerFactory, 5, connectionLimit.intValue());
  +            getLogger().debug("Using a bounded pool for RemoteManager handlers with upper limit " + connectionLimit.intValue());
  +        } else {
  +            // NOTE: The maximum here is not a real maximum.  The handler pool will continue to
  +            //       provide handlers beyond this value.
  +            theHandlerPool = new DefaultPool(theHandlerFactory, null, 5, 30);
  +            getLogger().debug("Using an unbounded pool for RemoteManager handlers.");
  +        }
  +        if (theHandlerPool instanceof LogEnabled) {
  +            ((LogEnabled)theHandlerPool).enableLogging(getLogger());
  +        }
  +        if (theHandlerPool instanceof Initializable) {
  +            ((Initializable)theHandlerPool).initialize();
           }
   
  -        final boolean useTLS = configuration.getChild( "useTLS" ).getValueAsBoolean( false );
  -        if( useTLS ) {
  -            m_serverSocketType = "ssl";
  +        theWatchdogFactory = new ThreadPerWatchdogFactory(threadPool, timeout);
  +        if (theWatchdogFactory instanceof LogEnabled) {
  +            ((LogEnabled)theWatchdogFactory).enableLogging(getLogger());
           }
  +    }
  +
  +    /**
  +     * @see org.apache.james.core.AbstractJamesService#getDefaultPort()
  +     */
  +     protected int getDefaultPort() {
  +        return 4555;
  +     }
   
  -        super.configure( configuration.getChild( "handler" ) );
  +    /**
  +     * @see org.apache.james.core.AbstractJamesService#getServiceType()
  +     */
  +    public String getServiceType() {
  +        return "Remote Manager Service";
       }
   
       /**
  -     * @see org.apache.avalon.framework.activity.Initializable#initialize()
  +     * @see org.apache.avalon.cornerstone.services.connection.AbstractHandlerFactory#newHandler()
        */
  -    public void initialize() throws Exception {
  -        getLogger().info( "RemoteManager init..." );
  -        StringBuffer infoBuffer =
  -            new StringBuffer(64)
  -                    .append("RemoteManager using ")
  -                    .append(m_serverSocketType)
  -                    .append(" on port ")
  -                    .append(m_port)
  -                    .append(" at ")
  -                    .append(m_bindTo);
  -        getLogger().info(infoBuffer.toString());
  -        super.initialize();
  -        getLogger().info("RemoteManager ...init end");
  +    protected ConnectionHandler newHandler()
  +            throws Exception {
  +        RemoteManagerHandler theHandler = (RemoteManagerHandler)theHandlerPool.get();
  +        theHandler.enableLogging(getLogger());
  +
  +        Watchdog theWatchdog = theWatchdogFactory.getWatchdog(theHandler.getWatchdogTarget());
  +
  +        theHandler.setConfigurationData(theConfigData);
  +        theHandler.setWatchdog(theWatchdog);
  +        return theHandler;
       }
   
       /**
  -     * @see org.apache.avalon.framework.activity.Disposable#dispose()
  +     * @see org.apache.avalon.cornerstone.services.connection.ConnectionHandlerFactory#releaseConnectionHandler(ConnectionHandler)
        */
  -    public void dispose()
  -    {
  -        getLogger().info( "RemoteManager dispose..." );
  -        getLogger().info( "RemoteManager dispose..." + m_connectionName);
  -        super.dispose();
  -       
  -        // This is needed to make sure that sockets are released promptly on Windows 2000
  -        System.gc();
  -    
  -        getLogger().info( "RemoteManager ...dispose end" );
  +    public void releaseConnectionHandler( ConnectionHandler connectionHandler ) {
  +        if (!(connectionHandler instanceof RemoteManagerHandler)) {
  +            throw new IllegalArgumentException("Attempted to return non-RemoteManagerHandler to pool.");
  +        }
  +        theHandlerPool.put((Poolable)connectionHandler);
  +    }
  +
  +    /**
  +     * The factory for producing handlers.
  +     */
  +    private static class RemoteManagerHandlerFactory
  +        implements ObjectFactory {
  +
  +        /**
  +         * @see org.apache.avalon.excalibur.pool.ObjectFactory#newInstance()
  +         */
  +        public Object newInstance() throws Exception {
  +            return new RemoteManagerHandler();
  +        }
  +
  +        /**
  +         * @see org.apache.avalon.excalibur.pool.ObjectFactory#getCreatedClass()
  +         */
  +        public Class getCreatedClass() {
  +            return RemoteManagerHandler.class;
  +        }
  +
  +        /**
  +         * @see org.apache.avalon.excalibur.pool.ObjectFactory#decommision(Object)
  +         */
  +        public void decommission( Object object ) throws Exception {
  +            return;
  +        }
  +    }
  +
  +    /**
  +     * A class to provide RemoteManager handler configuration to the handlers
  +     */
  +    private class RemoteManagerHandlerConfigurationDataImpl
  +        implements RemoteManagerHandlerConfigurationData {
  +
  +        /**
  +         * @see org.apache.james.remotemanager.RemoteManagerHandlerConfigurationData#getHelloName()
  +         */
  +        public String getHelloName() {
  +            return RemoteManager.this.helloName;
  +        }
  +
  +        /**
  +         * @see org.apache.james.remotemanager.RemoteManagerHandlerConfigurationData#getMailServer()
  +         */
  +        public MailServer getMailServer() {
  +            return RemoteManager.this.mailServer;
  +        }
  +
  +        /**
  +         * @see org.apache.james.remotemanager.RemoteManagerHandlerConfigurationData#getUsersRepository()
  +         */
  +        public UsersRepository getUsersRepository() {
  +            return RemoteManager.this.users;
  +        }
  +
  +        /**
  +         * @see org.apache.james.remotemanager.RemoteManagerHandlerConfigurationData#getUsersStore()
  +         */
  +        public UsersStore getUserStore() {
  +            return RemoteManager.this.usersStore;
  +        }
  +
  +        /**
  +         * @see org.apache.james.remotemanager.RemoteManagerHandlerConfigurationData#getAdministrativeAccountData()
  +         */
  +        public HashMap getAdministrativeAccountData() {
  +            return RemoteManager.this.adminAccounts;
  +        }
  +
       }
   }
  
  
  
  1.3       +3 -3      jakarta-james/src/java/org/apache/james/remotemanager/RemoteManager.xinfo
  
  Index: RemoteManager.xinfo
  ===================================================================
  RCS file: /home/cvs/jakarta-james/src/java/org/apache/james/remotemanager/RemoteManager.xinfo,v
  retrieving revision 1.2
  retrieving revision 1.3
  diff -u -r1.2 -r1.3
  --- RemoteManager.xinfo	25 Sep 2001 04:51:19 -0000	1.2
  +++ RemoteManager.xinfo	26 Oct 2002 04:15:30 -0000	1.3
  @@ -27,10 +27,10 @@
         <service name="org.apache.avalon.cornerstone.services.sockets.SocketManager" version="1.0"/>
       </dependency>
       <dependency>
  -      <service name="org.apache.avalon.cornerstone.services.scheduler.TimeScheduler" version="1.0"/>
  -    </dependency> 
  -    <dependency>
         <service name="org.apache.james.services.MailServer" version="1.0"/>
       </dependency> 
  +    <dependency>
  +      <service name="org.apache.avalon.cornerstone.services.threads.ThreadManager" version="1.0"/>
  +    </dependency>
     </dependencies>  
   </blockinfo>
  
  
  
  1.19      +117 -75   jakarta-james/src/java/org/apache/james/remotemanager/RemoteManagerHandler.java
  
  Index: RemoteManagerHandler.java
  ===================================================================
  RCS file: /home/cvs/jakarta-james/src/java/org/apache/james/remotemanager/RemoteManagerHandler.java,v
  retrieving revision 1.18
  retrieving revision 1.19
  diff -u -r1.18 -r1.19
  --- RemoteManagerHandler.java	2 Oct 2002 06:12:03 -0000	1.18
  +++ RemoteManagerHandler.java	26 Oct 2002 04:15:30 -0000	1.19
  @@ -8,25 +8,25 @@
   package org.apache.james.remotemanager;
   
   import org.apache.avalon.cornerstone.services.connection.ConnectionHandler;
  -import org.apache.avalon.cornerstone.services.scheduler.PeriodicTimeTrigger;
  -import org.apache.avalon.cornerstone.services.scheduler.Target;
  -import org.apache.avalon.cornerstone.services.scheduler.TimeScheduler;
  -import org.apache.avalon.framework.component.ComponentException;
  -import org.apache.avalon.framework.component.ComponentManager;
  -import org.apache.avalon.framework.component.Composable;
  +import org.apache.avalon.excalibur.pool.Poolable;
   import org.apache.avalon.framework.configuration.Configurable;
   import org.apache.avalon.framework.configuration.Configuration;
   import org.apache.avalon.framework.configuration.ConfigurationException;
  -import org.apache.james.BaseConnectionHandler;
  +import org.apache.avalon.framework.activity.Disposable;
  +import org.apache.avalon.framework.logger.AbstractLogEnabled;
   import org.apache.james.Constants;
   import org.apache.james.services.*;
   import org.apache.james.userrepository.DefaultUser;
  +import org.apache.james.util.watchdog.Watchdog;
  +import org.apache.james.util.watchdog.WatchdogTarget;
   import org.apache.mailet.MailAddress;
   
   import javax.mail.internet.ParseException;
   import java.io.BufferedReader;
  +import java.io.BufferedWriter;
   import java.io.IOException;
   import java.io.InputStreamReader;
  +import java.io.OutputStreamWriter;
   import java.io.PrintWriter;
   import java.net.Socket;
   import java.util.HashMap;
  @@ -44,13 +44,14 @@
    * @author  Federico Barbieri <sc...@pop.systemy.it>
    * @author <a href="mailto:donaldp@apache.org">Peter Donald</a>
    * @author <a href="mailto:charles@benett1.demon.co.uk">Charles Benett</a>
  + * @author Peter M. Goldstein <fa...@alum.mit.edu>
    *
    * @version $Revision$
    *
    */
   public class RemoteManagerHandler
  -    extends BaseConnectionHandler
  -    implements ConnectionHandler, Composable, Configurable, Target {
  +    extends AbstractLogEnabled
  +    implements ConnectionHandler, Poolable {
   
       /**
        * The text string for the ADDUSER command
  @@ -123,9 +124,9 @@
       private static final String COMMAND_SHUTDOWN = "SHUTDOWN";
   
       /**
  -     * The UsersStore that contains all UsersRepositories managed by this RemoteManager
  +     * The per-service configuration data that applies to all handlers
        */
  -    private UsersStore usersStore;
  +    private RemoteManagerHandlerConfigurationData theConfigData;
   
       /**
        * The current UsersRepository being managed/viewed/modified
  @@ -133,17 +134,6 @@
       private UsersRepository users;
   
       /**
  -     * The scheduler used to handle timeouts for the RemoteManager
  -     * interaction
  -     */
  -    private TimeScheduler scheduler;
  -
  -    /**
  -     * The reference to the internal MailServer service
  -     */
  -    private MailServer mailServer;
  -
  -    /**
        * Whether the local users repository should be used to store new
        * users.
        */
  @@ -166,38 +156,60 @@
       private Socket socket;
   
       /**
  -     * A HashMap of (user id, passwords) for James administrators
  +     * The watchdog being used by this handler to deal with idle timeouts.
        */
  -    private HashMap adminAccounts = new HashMap();
  +    private Watchdog theWatchdog;
   
       /**
  -     * @see org.apache.avalon.framework.component.Composable#compose(ComponentManager)
  +     * The watchdog target that idles out this handler.
  +     */
  +    private WatchdogTarget theWatchdogTarget = new RemoteManagerWatchdogTarget();
  +
  +    /**
  +     * Set the configuration data for the handler.
  +     *
  +     * @param theData the configuration data
        */
  -    public void compose( final ComponentManager componentManager )
  -        throws ComponentException {
  +    void setConfigurationData(RemoteManagerHandlerConfigurationData theData) {
  +        theConfigData = theData;
   
  -        scheduler = (TimeScheduler)componentManager.
  -            lookup( "org.apache.avalon.cornerstone.services.scheduler.TimeScheduler" );
  -        mailServer = (MailServer)componentManager.
  -            lookup( "org.apache.james.services.MailServer" );
  -        usersStore = (UsersStore)componentManager.
  -            lookup( "org.apache.james.services.UsersStore" );
  -        users = usersStore.getRepository("LocalUsers");;
  +        // Reset the users repository to the default.
  +        users = theConfigData.getUsersRepository();
  +        inLocalUsers = true;
       }
   
       /**
  -     * @see org.apache.avalon.framework.configuration.Configurable#configure(Configuration)
  +     * Set the Watchdog for use by this handler.
  +     *
  +     * @param theWatchdog the watchdog
        */
  -    public void configure( final Configuration configuration )
  -        throws ConfigurationException {
  +    void setWatchdog(Watchdog theWatchdog) {
  +        this.theWatchdog = theWatchdog;
  +    }
   
  -        timeout = configuration.getChild( "connectiontimeout" ).getValueAsInteger( 120000 );
  +    /**
  +     * Gets the Watchdog Target that should be used by Watchdogs managing
  +     * this connection.
  +     *
  +     * @return the WatchdogTarget
  +     */
  +    WatchdogTarget getWatchdogTarget() {
  +        return theWatchdogTarget;
  +    }
   
  -        final Configuration admin = configuration.getChild( "administrator_accounts" );
  -        final Configuration[] accounts = admin.getChildren( "account" );
  -        for ( int i = 0; i < accounts.length; i++ ) {
  -            adminAccounts.put( accounts[ i ].getAttribute( "login" ),
  -                               accounts[ i ].getAttribute( "password" ) );
  +    /**
  +     * Idle out this connection
  +     */
  +    void idleClose() {
  +        if (getLogger() != null) {
  +            getLogger().error("Remote Manager Connection has idled out.");
  +        }
  +        try {
  +            if (socket != null) {
  +                socket.close();
  +            }
  +        } catch (Exception e) {
  +            // ignored
           }
       }
   
  @@ -207,17 +219,14 @@
       public void handleConnection( final Socket connection )
           throws IOException {
   
  -        final PeriodicTimeTrigger trigger = new PeriodicTimeTrigger( timeout, -1 );
  -        scheduler.addTrigger( this.toString(), trigger, this );
           socket = connection;
           String remoteIP = socket.getInetAddress().getHostAddress();
           String remoteHost = socket.getInetAddress().getHostName();
   
           try {
  -            in = new BufferedReader(new InputStreamReader( socket.getInputStream() ));
  -            out = new PrintWriter( socket.getOutputStream(), true);
  -            if (getLogger().isInfoEnabled())
  -            {
  +            in = new BufferedReader(new InputStreamReader(socket.getInputStream(), "ASCII"), 512);
  +            out = new PrintWriter(new BufferedWriter(new OutputStreamWriter(socket.getOutputStream()), 512), false);
  +            if (getLogger().isInfoEnabled()) {
                   StringBuffer infoBuffer =
                       new StringBuffer(128)
                               .append("Access from ")
  @@ -227,12 +236,11 @@
                               .append(")");
                   getLogger().info( infoBuffer.toString() );
               }
  -            out.println( "JAMES RemoteAdministration Tool " + Constants.SOFTWARE_VERSION );
  +            out.println( "JAMES Remote Administration Tool " + Constants.SOFTWARE_VERSION );
               out.println("Please enter your login and password");
               String login = null;
               String password = null;
               do {
  -                scheduler.resetTrigger(this.toString());
                   if (login != null) {
                       final String message = "Login failed for " + login;
                       out.println( message );
  @@ -242,9 +250,7 @@
                   login = in.readLine().trim();
                   out.println("Password:");
                   password = in.readLine().trim();
  -            } while (!password.equals(adminAccounts.get(login)) || password.length() == 0);
  -
  -            scheduler.resetTrigger(this.toString());
  +            } while (!password.equals(theConfigData.getAdministrativeAccountData().get(login)) || password.length() == 0);
   
               StringBuffer messageBuffer =
                   new StringBuffer(64)
  @@ -252,8 +258,7 @@
                           .append(login)
                           .append(". HELP for a list of commands");
               out.println( messageBuffer.toString() );
  -            if (getLogger().isInfoEnabled())
  -            {
  +            if (getLogger().isInfoEnabled()) {
                   StringBuffer infoBuffer =
                       new StringBuffer(128)
                               .append("Login for ")
  @@ -263,14 +268,16 @@
               }
   
               try {
  +                theWatchdog.start();
                   while (parseCommand(in.readLine())) {
  -                    scheduler.resetTrigger(this.toString());
  +                    theWatchdog.reset();
                   }
  +                theWatchdog.stop();
               } catch (IOException ioe) {
                   //We can cleanly ignore this as it's probably a socket timeout
               } catch (Throwable thr) {
                   System.out.println("Exception: " + thr.getMessage());
  -                thr.printStackTrace();
  +                getLogger().error("Encountered exception in handling the remote manager connection.", thr);
               }
               StringBuffer infoBuffer =
                   new StringBuffer(64)
  @@ -278,7 +285,6 @@
                           .append(login)
                           .append(".");
               getLogger().info(infoBuffer.toString());
  -            socket.close();
   
           } catch ( final IOException e ) {
               out.println("Error. Closing connection");
  @@ -294,26 +300,45 @@
                               .append(e.getMessage());
                   getLogger().error(exceptionBuffer.toString());
               }
  +        } finally {
  +            resetHandler();
           }
  -
  -        scheduler.removeTrigger(this.toString());
       }
   
       /**
  -     * Callback method called when the the PeriodicTimeTrigger in 
  -     * handleConnection is triggered.  In this case the trigger is
  -     * being used as a timeout, so the method simply closes the connection.
  -     *
  -     * @param triggerName the name of the trigger
  +     * Resets the handler data to a basic state.
        */
  -    public void targetTriggered( final String triggerName ) {
  -        getLogger().error("Connection timeout on socket");
  +    private void resetHandler() {
  +
  +        // Clear the Watchdog
  +        if (theWatchdog != null) {
  +            if (theWatchdog instanceof Disposable) {
  +                ((Disposable)theWatchdog).dispose();
  +            }
  +            theWatchdog = null;
  +        }
  +
  +        in = null;
  +        out = null;
           try {
  -            out.println("Connection timeout. Closing connection");
  -            socket.close();
  -        } catch ( final IOException ioe ) {
  -            // Ignored
  +            if (socket != null) {
  +                socket.close();
  +            }
  +        } catch (IOException e) {
  +            if (getLogger().isErrorEnabled()) {
  +                getLogger().error("Exception closing socket: "
  +                                  + e.getMessage());
  +            }
  +        } finally {
  +            socket = null;
           }
  +
  +        // Reset user repository
  +        users = theConfigData.getUsersRepository();
  +        inLocalUsers = true;
  +
  +        // Clear config data
  +        theConfigData = null;
       }
   
       /**
  @@ -407,7 +432,7 @@
           } else if ( inLocalUsers ) {
               // TODO: Why does the LocalUsers repository get treated differently?
               //       What exactly is the LocalUsers repository?
  -            success = mailServer.addUser(username, passwd);
  +            success = theConfigData.getMailServer().addUser(username, passwd);
           } else {
               DefaultUser user = new DefaultUser(username, "SHA");
               user.setPassword(passwd);
  @@ -800,7 +825,7 @@
               return true;
           }
           String repositoryName = argument.toLowerCase(Locale.US);
  -        UsersRepository repos = usersStore.getRepository(repositoryName);
  +        UsersRepository repos = theConfigData.getUserStore().getRepository(repositoryName);
           if ( repos == null ) {
               out.println("No such repository: " + repositoryName);
           } else {
  @@ -856,6 +881,23 @@
           out.println("Unknown command " + argument);
           out.flush();
           return true;
  +    }
  +
  +    /**
  +     * A private inner class which serves as an adaptor
  +     * between the WatchdogTarget interface and this
  +     * handler class.
  +     */
  +    private class RemoteManagerWatchdogTarget
  +        implements WatchdogTarget {
  +
  +        /**
  +         * @see org.apache.james.util.watchdog.WatchdogTarget#execute()
  +         */
  +        public void execute() {
  +            RemoteManagerHandler.this.idleClose();
  +        }
  +
       }
   
   }
  
  
  
  1.1                  jakarta-james/src/java/org/apache/james/remotemanager/RemoteManagerHandlerConfigurationData.java
  
  Index: RemoteManagerHandlerConfigurationData.java
  ===================================================================
  /*
   * Copyright (C) The Apache Software Foundation. All rights reserved.
   *
   * This software is published under the terms of the Apache Software License
   * version 1.1, a copy of which has been included with this distribution in
   * the LICENSE file.
   */
  package org.apache.james.remotemanager;
  
  import org.apache.james.services.MailServer;
  import org.apache.james.services.UsersRepository;
  import org.apache.james.services.UsersStore;
  
  import java.util.HashMap;
  
  /**
   * Provides a number of server-wide constant values to the
   * RemoteManagerHandlers
   *
   * @author Peter M. Goldstein <fa...@alum.mit.edu>
   */
  public interface RemoteManagerHandlerConfigurationData {
  
      /**
       * Returns the service wide hello name
       *
       * @return the hello name
       */
      String getHelloName();
  
      /**
       * Returns the MailServer interface for this service.
       *
       * @return the MailServer interface for this service
       */
      MailServer getMailServer();
  
      /**
       * Returns the UsersRepository for this service.
       *
       * @return the local users repository
       */
      UsersRepository getUsersRepository();
  
      /**
       * Returns the UsersStore for this service.
       *
       * @return the local users store
       */
      UsersStore getUserStore();
  
      /**
       * Returns the Administrative Account Data
       *
       * TODO: Change the return type to make this immutable.
       *
       * @return the admin account data
       */
      HashMap getAdministrativeAccountData();
  
  }
  
  
  
  1.17      +37 -5     jakarta-james/src/java/org/apache/james/core/MailImpl.java
  
  Index: MailImpl.java
  ===================================================================
  RCS file: /home/cvs/jakarta-james/src/java/org/apache/james/core/MailImpl.java,v
  retrieving revision 1.16
  retrieving revision 1.17
  diff -u -r1.16 -r1.17
  --- MailImpl.java	24 Sep 2002 13:27:09 -0000	1.16
  +++ MailImpl.java	26 Oct 2002 04:15:30 -0000	1.17
  @@ -6,6 +6,9 @@
    * the LICENSE file.
    */
   package org.apache.james.core;
  +
  +import org.apache.avalon.framework.activity.Disposable;
  +
   import org.apache.james.util.RFC2822Headers;
   import org.apache.mailet.Mail;
   import org.apache.mailet.MailAddress;
  @@ -17,11 +20,13 @@
   import javax.mail.internet.MimeMessage;
   import javax.mail.internet.ParseException;
   import java.io.*;
  +import java.util.ArrayList;
   import java.util.Collection;
   import java.util.Date;
   import java.util.Enumeration;
   import java.util.HashSet;
  -import java.util.Vector;
  +import java.util.Iterator;
  +
   /**
    * Wraps a MimeMessage adding routing information (from SMTP) and some simple
    * API enhancements.
  @@ -30,7 +35,7 @@
    * @author Stuart Roebuck <st...@adolos.co.uk>
    * @version 0.9
    */
  -public class MailImpl implements Mail {
  +public class MailImpl implements Disposable, Mail {
       /**
        * We hardcode the serialVersionUID so that from James 1.2 on,
        * MailImpl will be deserializable (so your mail doesn't get lost)
  @@ -90,8 +95,18 @@
           this();
           this.name = name;
           this.sender = sender;
  -        this.recipients = recipients;
  +        this.recipients = null;
  +
  +        // Copy the recipient list
  +        if (recipients != null) {
  +            Iterator theIterator = recipients.iterator();
  +            this.recipients = new ArrayList();
  +            while (theIterator.hasNext()) {
  +                this.recipients.add(theIterator.next());
  +            }
  +        }
       }
  +
       /**
        * A constructor that creates a MailImpl with the specified name,
        * sender, recipients, and message data.
  @@ -108,6 +123,7 @@
           MimeMessageWrapper wrapper = new MimeMessageWrapper(source);
           this.setMessage(wrapper);
       }
  +
       /**
        * A constructor that creates a MailImpl with the specified name,
        * sender, recipients, and MimeMessage.
  @@ -121,16 +137,17 @@
           this(name, sender, recipients);
           this.setMessage(message);
       }
  +
       /**
        * A constructor which will attempt to obtain sender and recipients from the headers of the MimeMessage supplied.
        * @param message - a MimeMessage from which to construct a Mail
        */
       public MailImpl(MimeMessage message) throws MessagingException {
  -       this();
  +        this();
           Address[] addresses;
           addresses = message.getFrom();
           MailAddress sender = new MailAddress(new InternetAddress(addresses[0].toString()));
  -        Collection recipients = new Vector();
  +        Collection recipients = new ArrayList();
           addresses = message.getRecipients(MimeMessage.RecipientType.TO);
           for (int i = 0; i < addresses.length; i++) {
               recipients.add(new MailAddress(new InternetAddress(addresses[i].toString())));
  @@ -467,4 +484,19 @@
           out.writeObject(remoteAddr);
           out.writeObject(lastUpdated);
       }
  +
  +    /**
  +     * @see org.apache.avalon.framework.activity.Disposable#dispose()
  +     */
  +    public void dispose() {
  +        try {
  +            MimeMessage wrapper = getMessage();
  +            if (wrapper instanceof Disposable) {
  +                ((Disposable)wrapper).dispose();
  +            }
  +        } catch (MessagingException me) {
  +            // Ignored
  +        }
  +    }
  +
   }
  
  
  
  1.9       +17 -6     jakarta-james/src/java/org/apache/james/core/MimeMessageInputStreamSource.java
  
  Index: MimeMessageInputStreamSource.java
  ===================================================================
  RCS file: /home/cvs/jakarta-james/src/java/org/apache/james/core/MimeMessageInputStreamSource.java,v
  retrieving revision 1.8
  retrieving revision 1.9
  diff -u -r1.8 -r1.9
  --- MimeMessageInputStreamSource.java	3 Sep 2002 16:52:16 -0000	1.8
  +++ MimeMessageInputStreamSource.java	26 Oct 2002 04:15:30 -0000	1.9
  @@ -10,6 +10,8 @@
   import java.io.*;
   import javax.mail.MessagingException;
   
  +import org.apache.avalon.framework.activity.Disposable;
  +
   /**
    * Takes an input stream and creates a repeatable input stream source
    * for a MimeMessageWrapper.  It does this by completely reading the
  @@ -21,7 +23,9 @@
    *
    * @author <a href="mailto:sergek@lokitech.com>">Serge Knystautas</a>
    */
  -public class MimeMessageInputStreamSource extends MimeMessageSource {
  +public class MimeMessageInputStreamSource
  +    extends MimeMessageSource
  +    implements Disposable {
   
       /**
        * A temporary file used to hold the message stream
  @@ -57,7 +61,6 @@
                   fout.write(b);
               }
               fout.flush();
  -            file.deleteOnExit();
   
               sourceId = file.getCanonicalPath();
           } catch (IOException ioe) {
  @@ -111,11 +114,9 @@
       }
   
       /**
  -     * <p>Finalizer that closes and deletes the temp file.  Very bad.</p>
  -     * <p>TODO: Should be replaced with a more robust cleanup mechanism</p>
  -     *
  +     * @see org.apache.avalon.framework.activity.Disposable#dispose()
        */
  -    public void finalize() {
  +    public void dispose() {
           try {
               if (file != null && file.exists()) {
                   file.delete();
  @@ -124,5 +125,15 @@
               //ignore
           }
           file = null;
  +    }
  +
  +    /**
  +     * <p>Finalizer that closes and deletes the temp file.  Very bad.</p>
  +     * We're leaving this in temporarily, while also establishing a more
  +     * formal mechanism for cleanup through use of the dispose() method.
  +     *
  +     */
  +    public void finalize() {
  +        dispose();
       }
   }
  
  
  
  1.8       +1 -0      jakarta-james/src/java/org/apache/james/core/MimeMessageSource.java
  
  Index: MimeMessageSource.java
  ===================================================================
  RCS file: /home/cvs/jakarta-james/src/java/org/apache/james/core/MimeMessageSource.java,v
  retrieving revision 1.7
  retrieving revision 1.8
  diff -u -r1.7 -r1.8
  --- MimeMessageSource.java	17 Aug 2002 18:16:24 -0000	1.7
  +++ MimeMessageSource.java	26 Oct 2002 04:15:30 -0000	1.8
  @@ -66,4 +66,5 @@
           }
           return size;
       }
  +
   }
  
  
  
  1.17      +28 -10    jakarta-james/src/java/org/apache/james/core/MimeMessageWrapper.java
  
  Index: MimeMessageWrapper.java
  ===================================================================
  RCS file: /home/cvs/jakarta-james/src/java/org/apache/james/core/MimeMessageWrapper.java,v
  retrieving revision 1.16
  retrieving revision 1.17
  diff -u -r1.16 -r1.17
  --- MimeMessageWrapper.java	30 Sep 2002 02:26:44 -0000	1.16
  +++ MimeMessageWrapper.java	26 Oct 2002 04:15:30 -0000	1.17
  @@ -16,15 +16,21 @@
   import java.util.Enumeration;
   import java.util.Vector;
   
  +import org.apache.james.util.InternetPrintWriter;
   import org.apache.james.util.RFC2822Headers;
   import org.apache.james.util.RFC822DateFormat;
   
  +import org.apache.avalon.framework.activity.Disposable;
  +
  +
   /**
    * This object wraps a MimeMessage, only loading the underlying MimeMessage
    * object when needed.  Also tracks if changes were made to reduce
    * unnecessary saves.
    */
  -public class MimeMessageWrapper extends MimeMessage {
  +public class MimeMessageWrapper
  +    extends MimeMessage
  +    implements Disposable {
   
       /**
        * Can provide an input stream to the data
  @@ -164,6 +170,9 @@
           if (message == null || !isModified()) {
               //We do not want to instantiate the message... just read from source
               //  and write to this outputstream
  +
  +            // TODO: This is really a bad way to do this sort of thing.  A shared buffer to 
  +            //       allow simultaneous read/writes would be a substantial improvement
               byte[] block = new byte[1024];
               int read = 0;
               InputStream in = source.getInputStream();
  @@ -199,14 +208,14 @@
               //First handle the headers
               InputStream in = source.getInputStream();
               InternetHeaders headers = new InternetHeaders(in);
  -            PrintStream pos = new PrintStream(headerOs);
  +            PrintWriter pos = new InternetPrintWriter(new BufferedWriter(new OutputStreamWriter(headerOs), 512), true);
               for (Enumeration e = headers.getNonMatchingHeaderLines(ignoreList); e.hasMoreElements(); ) {
                   String header = (String)e.nextElement();
  -                pos.print(header);
  -                pos.print("\r\n");
  +                pos.println(header);
               }
  -            pos.print("\r\n");
  -            pos.flush();
  +            pos.println();
  +            // TODO: This is really a bad way to do this sort of thing.  A shared buffer to 
  +            //       allow simultaneous read/writes would be a substantial improvement
               byte[] block = new byte[1024];
               int read = 0;
               while ((read = in.read(block)) > 0) {
  @@ -239,12 +248,10 @@
   
               //Write the headers (minus ignored ones)
               Enumeration headers = message.getNonMatchingHeaderLines(ignoreList);
  -            PrintStream pos = new PrintStream(headerOs);
  +            PrintWriter pos = new InternetPrintWriter(new BufferedWriter(new OutputStreamWriter(headerOs), 512), true);
               while (headers.hasMoreElements()) {
  -                pos.print((String)headers.nextElement());
  -                pos.print("\r\n");
  +                pos.println((String)headers.nextElement());
               }
  -            pos.flush();
   
               //Write the message without any headers
   
  @@ -623,6 +630,8 @@
               loadMessage();
           }
           InputStream in = getContentStream();
  +        // TODO: This is really a bad way to do this sort of thing.  A shared buffer to 
  +        //       allow simultaneous read/writes would be a substantial improvement
           byte block[] = new byte[1024];
           int len = 0;
           while ((len = in.read(block)) > -1) {
  @@ -886,6 +895,15 @@
           }
           modified = true;
           message.setRecipients(type, addresses);
  +    }
  +
  +    /**
  +     * @see org.apache.avalon.framework.activity.Disposable#dispose()
  +     */
  +    public void dispose() {
  +        if (source instanceof Disposable) {
  +            ((Disposable)source).dispose();
  +        }
       }
   
   }
  
  
  
  1.1                  jakarta-james/src/java/org/apache/james/core/AbstractJamesService.java
  
  Index: AbstractJamesService.java
  ===================================================================
  /*
   * Copyright (C) The Apache Software Foundation. All rights reserved.
   *
   * This software is published under the terms of the Apache Software License
   * version 1.1, a copy of which has been included with this distribution in
   * the LICENSE file.
   */
  package org.apache.james.core;
  
  import java.io.*;
  import java.net.*;
  
  import org.apache.avalon.framework.logger.*;
  import org.apache.avalon.framework.component.*;
  import org.apache.avalon.framework.configuration.*;
  import org.apache.avalon.framework.activity.*;
  
  import org.apache.avalon.excalibur.thread.ThreadPool;
  import org.apache.avalon.cornerstone.services.threads.ThreadManager;
  
  import org.apache.avalon.cornerstone.services.connection.AbstractHandlerFactory;
  import org.apache.avalon.cornerstone.services.connection.ConnectionHandler;
  import org.apache.avalon.cornerstone.services.connection.ConnectionHandlerFactory;
  import org.apache.avalon.cornerstone.services.connection.ConnectionManager;
  import org.apache.avalon.cornerstone.services.sockets.ServerSocketFactory;
  import org.apache.avalon.cornerstone.services.sockets.SocketManager;
  
  import org.apache.james.util.connection.SimpleConnectionManager;
  
  /**
   * Server which creates connection handlers. All new James servers must
   * inherit from this abstract server by implementing its
   *
   * @author <a href="mailto:myfam@surfeu.fi">Andrei Ivanov</a>
   * @author <a href="farsight@alum.mit.edu">Peter M. Goldstein</a>
   */
  public abstract class AbstractJamesService extends AbstractHandlerFactory
      implements Component, Composable, Configurable,
                 Disposable, Initializable, ConnectionHandlerFactory {
  
      public static final int DEFAULT_TIMEOUT = 5* 60 * 1000;
  
      public static final String TIMEOUT_NAME = "connectiontimeout";
  
      public static final String HELLO_NAME = "helloName";
  
      /**
       * The ConnectionManager that spawns and manages service connections.
       */
      private ConnectionManager connectionManager;
  
      /**
       * The name of the thread group to be used by this service for 
       * generating connections
       */
      protected String threadGroup;
  
      /**
       * The thread pool used by this service that holds the threads
       * that service the client connections.
       */
      protected ThreadPool threadPool = null;
  
      /**
       * The server socket type used to generate connections for this server.
       */
      protected String serverSocketType = "plain";
  
      /**
       * The port on which this service will be made available.
       */
      protected int port = -1;
  
      /**
       * Network interface to which the service will bind.  If not set,
       * the server binds to all available interfaces.
       */
      protected InetAddress bindTo = null;
  
      /*
       * The server socket associated with this service
       */
      protected ServerSocket serverSocket;
  
      /**
       * The name of the connection used by this service.  We need to
       * track this so we can tell the ConnectionManager which service
       * to disconnect upon shutdown.
       */
      protected String connectionName;
  
      /**
       * The maximum number of connections allowed for this service.
       */
      protected Integer connectionLimit;
  
      /**
       * The connection idle timeout.  Used primarily to prevent server
       * problems from hanging a connection.
       */
      protected int timeout;
  
      /**
       * The hello name for the service.
       */
      protected String helloName;
  
      /**
       * The component manager used by this service.
       */
      private ComponentManager compMgr;
  
      /**
       * Whether this service is enabled.
       */
      private volatile boolean enabled;
  
      /**
       * @see org.apache.avalon.framework.component.Composable#compose(ComponentManager)
       */
      public void compose(ComponentManager comp) throws ComponentException {
          super.compose(comp);
          compMgr = comp;
          connectionManager = (ConnectionManager) compMgr.lookup(ConnectionManager.ROLE);
      }
  
      /**
       * @see org.apache.avalon.framework.configuration.Configurable#configure(Configuration)
       */
      public void configure(Configuration conf) throws ConfigurationException {
          enabled = conf.getAttributeAsBoolean("enabled", true);
          if (!enabled) {
            getLogger().info(getServiceType() + " disabled by configuration");
            return;
          }
  
          Configuration handlerConfiguration = conf.getChild("handler");
  
          // Send the handler subconfiguration to the super class.  This 
          // ensures that the handler config is passed to the handlers.
          //
          // TODO: This should be rationalized.  The handler element of the
          //       server configuration doesn't really make a whole lot of 
          //       sense.  We should modify the config to get rid of it.
          //       Keeping it for now to maintain backwards compatibility.
          super.configure(handlerConfiguration);
  
          port = conf.getChild("port").getValueAsInteger(getDefaultPort());
  
          Configuration serverSocketTypeConf = conf.getChild("server-socket-type", false);
          String confSocketType = null;
          if (serverSocketTypeConf != null ) {
              confSocketType = serverSocketTypeConf.getValue();
          }
  
          if (confSocketType == null) {
              // Only load the useTLS parameter if a specific socket type has not
              // been specified.  This maintains backwards compatibility while
              // allowing us to have more complex (i.e. multiple SSL configuration)
              // deployments
              final boolean useTLS = conf.getChild("useTLS").getValueAsBoolean(isDefaultTLSEnabled());
              if (useTLS) {
                serverSocketType = "ssl";
              }
          } else {
              serverSocketType = confSocketType;
          }
  
          StringBuffer infoBuffer;
          threadGroup = conf.getChild("thread-group").getValue(null);
          if (threadGroup != null) {
              infoBuffer =
                  new StringBuffer(64)
                          .append(getServiceType())
                          .append(" uses thread group: ")
                          .append(threadGroup);
              getLogger().info(infoBuffer.toString());
          }
          else {
              getLogger().info(getServiceType() + " uses default thread group.");
          }
  
          try {
              final String bindAddress = conf.getChild("bind").getValue(null);
              if( null != bindAddress ) {
                  bindTo = InetAddress.getByName(bindAddress);
                  infoBuffer =
                      new StringBuffer(64)
                              .append(getServiceType())
                              .append(" bound to: ")
                              .append(bindTo);
                  getLogger().info(infoBuffer.toString());
              }
          }
          catch( final UnknownHostException unhe ) {
              throw new ConfigurationException( "Malformed bind parameter in configuration of service " + getServiceType(), unhe );
          }
  
          String hostName = null;
          try {
              hostName = InetAddress.getLocalHost().getHostName();
          } catch (UnknownHostException ue) {
              hostName = "localhost";
          }
  
          infoBuffer =
              new StringBuffer(64)
                      .append(getServiceType())
                      .append(" is running on: ")
                      .append(hostName);
          getLogger().info(infoBuffer.toString());
  
          Configuration helloConf = handlerConfiguration.getChild("helloName");
          boolean autodetect = helloConf.getAttributeAsBoolean("autodetect", true);
          if (autodetect) {
              helloName = hostName;
          } else {
              helloName = helloConf.getValue("localhost");
          }
          infoBuffer =
              new StringBuffer(64)
                      .append(getServiceType())
                      .append(" handler hello name is: ")
                      .append(helloName);
          getLogger().info(infoBuffer.toString());
  
          timeout = handlerConfiguration.getChild(TIMEOUT_NAME).getValueAsInteger(DEFAULT_TIMEOUT);
  
          infoBuffer =
              new StringBuffer(64)
                      .append(getServiceType())
                      .append(" handler connection timeout is: ")
                      .append(timeout);
          getLogger().info(infoBuffer.toString());
  
          final String location = "generated:" + getServiceType();
  
          if (connectionManager instanceof SimpleConnectionManager) {
              String connectionLimitString = conf.getChild("connection-limit").getValue(null);
              if (connectionLimitString != null) {
                  try {
                      connectionLimit = new Integer(connectionLimitString);
                  } catch (NumberFormatException nfe) {
                      getLogger().error("Connection limit value is not properly formatted.", nfe);
                  }
                  if (connectionLimit.intValue() < 0) {
                      getLogger().error("Connection limit value cannot be less than zero.");
                      throw new ConfigurationException("Connection limit value cannot be less than zero.");
                  }
              } else {
                  connectionLimit = new Integer(((SimpleConnectionManager)connectionManager).getMaximumNumberOfOpenConnections());
              }
              infoBuffer = new StringBuffer(128)
                  .append(getServiceType())
                  .append(" will allow a maximum of ")
                  .append(connectionLimit.intValue())
                  .append(" connections.");
              getLogger().info(infoBuffer.toString());
          }
  
  
      }
  
      /**
       * @see org.apache.avalon.framework.activity.Initializable#initialize()
       */
      public void initialize() throws Exception {
          if (!isEnabled()) {
              getLogger().info(getServiceType() + " Disabled");
              System.out.println(getServiceType() + " Disabled");
              return;
          }
          getLogger().debug(getServiceType() + " init...");
  
          SocketManager socketManager = (SocketManager) compMgr.lookup(SocketManager.ROLE);
  
          ThreadManager threadManager = (ThreadManager) compMgr.lookup(ThreadManager.ROLE);
  
          if (threadGroup != null) {
              threadPool = threadManager.getThreadPool(threadGroup);
          } else {
              threadPool = threadManager.getDefaultThreadPool();
          }
  
          ServerSocketFactory factory = socketManager.getServerSocketFactory(serverSocketType);
          ServerSocket serverSocket = factory.createServerSocket(port, 5, bindTo);
      
          if (null == connectionName) {
              final StringBuffer sb = new StringBuffer();
              sb.append(serverSocketType);
              sb.append(':');
              sb.append(port);
      
              if (null != bindTo) {
                  sb.append('/');
                  sb.append(bindTo);
              }
              connectionName = sb.toString();
          }
  
          if ((connectionLimit != null) &&
              (connectionManager instanceof SimpleConnectionManager)) {
              if (null != threadPool) {
                  ((SimpleConnectionManager)connectionManager).connect(connectionName, serverSocket, this, threadPool, connectionLimit.intValue());
              }
              else {
                  ((SimpleConnectionManager)connectionManager).connect(connectionName, serverSocket, this, connectionLimit.intValue()); // default pool
              }
          } else {
              if (null != threadPool) {
                  connectionManager.connect(connectionName, serverSocket, this, threadPool);
              }
              else {
                  connectionManager.connect(connectionName, serverSocket, this); // default pool
              }
          }
  
          getLogger().debug(getServiceType() + " ...init end");
  
          StringBuffer logBuffer =
              new StringBuffer(64)
                  .append(getServiceType())
                  .append(" started ")
                  .append(connectionName);
          String logString = logBuffer.toString();
          System.out.println(logString);
          getLogger().info(logString);
      }
  
      /**
       * @see org.apache.avalon.framework.activity.Disposable#dispose()
       */
      public void dispose() {
  
          if (!isEnabled()) {
              return;
          }
          StringBuffer infoBuffer =
              new StringBuffer(64)
                      .append(getServiceType())
                      .append(" dispose... ")
                      .append(connectionName);
          getLogger().debug(infoBuffer.toString());
  
          try {
              connectionManager.disconnect(connectionName, true);
          } catch (final Exception e) {
              StringBuffer warnBuffer =
                  new StringBuffer(64)
                          .append("Error disconnecting ")
                          .append(getServiceType())
                          .append(": ");
              getLogger().warn(warnBuffer.toString(), e);
          }
  
          compMgr = null;
  
          connectionManager = null;
          threadPool = null;
  
          // This is needed to make sure sockets are promptly closed on Windows 2000
          // TODO: Check this - shouldn't need to explicitly gc to force socket closure
          System.gc();
      
          getLogger().debug(getServiceType() + " ...dispose end");
      }
  
      /**
       * Describes whether this service is enabled by configuration.
       *
       * @return is the service enabled.
       */
      protected final boolean isEnabled() {
          return enabled;
      }
      /**
       * Overide this method to create actual instance of connection handler.
       *
       * @return the new ConnectionHandler
       * @exception Exception if an error occurs
       */
      protected abstract ConnectionHandler newHandler()
          throws Exception;
  
      /**
       * Get the default port for this server type.
       *
       * It is strongly recommended that subclasses of this class
       * override this method to specify the default port for their
       * specific server type.
       *
       * @return the default port
       */
       protected int getDefaultPort() {
          return 0;
       }
  
      /**
       * Get whether TLS is enabled for this server's socket by default.
       *
       * @return the default port
       */
       protected boolean isDefaultTLSEnabled() {
          return false;
       }
  
      /**
       * This method returns the type of service provided by this server.
       * This should be invariant over the life of the class.
       *
       * Subclasses may override this implementation.  This implementation
       * parses the complete class name and returns the undecorated class
       * name.
       *
       * @return description of this server
       */
      public String getServiceType() {
          String name = getClass().getName();
          int p = name.lastIndexOf(".");
          if (p > 0 && p < name.length() - 2) {
              name = name.substring(p + 1);
          }
          return name;
      }
  }
  
  
  
  
  1.32      +370 -284  jakarta-james/src/java/org/apache/james/smtpserver/SMTPHandler.java
  
  Index: SMTPHandler.java
  ===================================================================
  RCS file: /home/cvs/jakarta-james/src/java/org/apache/james/smtpserver/SMTPHandler.java,v
  retrieving revision 1.31
  retrieving revision 1.32
  diff -u -r1.31 -r1.32
  --- SMTPHandler.java	17 Oct 2002 22:04:01 -0000	1.31
  +++ SMTPHandler.java	26 Oct 2002 04:15:30 -0000	1.32
  @@ -8,23 +8,18 @@
   package org.apache.james.smtpserver;
   
   import org.apache.avalon.cornerstone.services.connection.ConnectionHandler;
  -import org.apache.avalon.cornerstone.services.scheduler.PeriodicTimeTrigger;
  -import org.apache.avalon.cornerstone.services.scheduler.Target;
  -import org.apache.avalon.cornerstone.services.scheduler.TimeScheduler;
  -import org.apache.avalon.framework.component.ComponentException;
  -import org.apache.avalon.framework.component.ComponentManager;
  -import org.apache.avalon.framework.component.Composable;
  -import org.apache.avalon.framework.configuration.Configurable;
  -import org.apache.avalon.framework.configuration.Configuration;
  -import org.apache.avalon.framework.configuration.ConfigurationException;
  -import org.apache.james.BaseConnectionHandler;
  +import org.apache.avalon.excalibur.pool.Poolable;
  +import org.apache.avalon.framework.activity.Disposable;
  +import org.apache.avalon.framework.logger.AbstractLogEnabled;
   import org.apache.james.Constants;
   import org.apache.james.core.MailHeaders;
   import org.apache.james.core.MailImpl;
   import org.apache.james.services.MailServer;
   import org.apache.james.services.UsersRepository;
  -import org.apache.james.services.UsersStore;
   import org.apache.james.util.*;
  +import org.apache.james.util.watchdog.BytesReadResetInputStream;
  +import org.apache.james.util.watchdog.Watchdog;
  +import org.apache.james.util.watchdog.WatchdogTarget;
   import org.apache.mailet.MailAddress;
   import javax.mail.MessagingException;
   import java.io.*;
  @@ -45,8 +40,8 @@
    * @version This is $Revision$
    */
   public class SMTPHandler
  -    extends BaseConnectionHandler
  -    implements ConnectionHandler, Composable, Configurable, Target {
  +    extends AbstractLogEnabled
  +    implements ConnectionHandler, Poolable {
   
       /**
        * SMTP Server identification string used in SMTP headers
  @@ -60,7 +55,7 @@
       private final static String SENDER = "SENDER_ADDRESS";     // Sender's email address 
       private final static String MESG_FAILED = "MESG_FAILED";   // Message failed flag
       private final static String MESG_SIZE = "MESG_SIZE";       // The size of the message
  -    private final static String RCPT_VECTOR = "RCPT_VECTOR";   // The message recipients
  +    private final static String RCPT_LIST = "RCPT_LIST";   // The message recipients
   
       /**
        * The character array that indicates termination of an SMTP connection
  @@ -152,7 +147,6 @@
        */
       private final static String MAIL_OPTION_SIZE = "SIZE";
   
  -
       /**
        * The TCP/IP socket over which the SMTP 
        * dialogue is occurring.
  @@ -195,103 +189,95 @@
       private String smtpID;
   
       /**
  -     * Whether authentication is required to use
  -     * this SMTP server.
  +     * The per-service configuration data that applies to all handlers
        */
  -    private boolean authRequired = false;
  -
  +    private SMTPHandlerConfigurationData theConfigData;
   
       /**
  -     * Whether the server verifies that the user
  -     * actually sending an email matches the
  -     * authentication credentials attached to the
  -     * SMTP interaction.
  +     * The hash map that holds variables for the SMTP message transfer in progress.
  +     *
  +     * This hash map should only be used to store variable set in a particular
  +     * set of sequential MAIL-RCPT-DATA commands, as described in RFC 2821.  Per
  +     * connection values should be stored as member variables in this class.
        */
  -    private boolean verifyIdentity = false;
  -
  +    private HashMap state = new HashMap();
   
       /**
  -     * The maximum message size allowed by this SMTP server.  The default
  -     * value, 0, means no limit.
  +     * The watchdog being used by this handler to deal with idle timeouts.
        */
  -    private long maxmessagesize = 0;
  +    Watchdog theWatchdog;
   
       /**
  -     * The number of bytes to read before resetting the connection timeout
  -     * timer.  Defaults to 20,000 bytes.
  +     * The watchdog target that idles out this handler.
        */
  -    private int lengthReset = 20000;
  -                                    
  +    WatchdogTarget theWatchdogTarget = new SMTPWatchdogTarget();
  +
       /**
  -     * The scheduler used to handle timeouts for the SMTP interaction.
  +     * The per-handler response buffer used to marshal responses.
        */
  -    private TimeScheduler scheduler;
  +    StringBuffer responseBuffer = new StringBuffer(256);
  +
  +        public SMTPHandler() {
  +            System.out.println("Creating SMTPHandler");
  +        }
  +
   
       /**
  -     * The user repository for this server - used to authenticate
  -     * users.
  +     * Set the configuration data for the handler
  +     *
  +     * @param theData the per-service configuration data for this handler
        */
  -    private UsersRepository users;
  +    void setConfigurationData(SMTPHandlerConfigurationData theData) {
  +        theConfigData = theData;
  +    }
   
       /**
  -     * The internal mail server service.
  +     * Set the Watchdog for use by this handler.
  +     *
  +     * @param theWatchdog the watchdog
        */
  -    private MailServer mailServer;
  +    void setWatchdog(Watchdog theWatchdog) {
  +        this.theWatchdog = theWatchdog;
  +    }
   
       /**
  -     * The hash map that holds variables for the SMTP message transfer in progress.
  +     * Gets the Watchdog Target that should be used by Watchdogs managing
  +     * this connection.
        *
  -     * This hash map should only be used to store variable set in a particular
  -     * set of sequential MAIL-RCPT-DATA commands, as described in RFC 2821.  Per
  -     * connection values should be stored as member variables in this class.
  +     * @return the WatchdogTarget
        */
  -    private HashMap state = new HashMap();
  +    WatchdogTarget getWatchdogTarget() {
  +        return theWatchdogTarget;
  +    }
   
       /**
  -     * @see org.apache.avalon.framework.component.Composable#compose(ComponentManager)
  +     * Idle out this connection
        */
  -    public void compose(final ComponentManager componentManager) throws ComponentException {
  -        mailServer = (MailServer) componentManager.lookup("org.apache.james.services.MailServer");
  -        scheduler =
  -            (TimeScheduler) componentManager.lookup(
  -                "org.apache.avalon.cornerstone.services.scheduler.TimeScheduler");
  -        UsersStore usersStore =
  -            (UsersStore) componentManager.lookup("org.apache.james.services.UsersStore");
  -        users = usersStore.getRepository("LocalUsers");
  -     }
  -
  -    /**
  -     * Pass the <code>Configuration</code> to the instance.
  -     *
  -     * @param configuration the class configurations.
  -     * @throws ConfigurationException if an error occurs
  -     */
  -    public void configure(Configuration configuration) throws ConfigurationException {
  -        super.configure(configuration);
  -        authRequired = configuration.getChild("authRequired").getValueAsBoolean(false);
  -        verifyIdentity = configuration.getChild("verifyIdentity").getValueAsBoolean(false);
  -        // get the message size limit from the conf file and multiply
  -        // by 1024, to put it in bytes
  -        maxmessagesize = configuration.getChild( "maxmessagesize" ).getValueAsLong( 0 ) * 1024;
  -        if (getLogger().isDebugEnabled()) {
  -            getLogger().debug("Max message size is: " + maxmessagesize);
  +    void idleClose() {
  +        if (getLogger() != null) {
  +            getLogger().error("SMTP Connection has idled out.");
  +        }
  +        try {
  +            if (socket != null) {
  +                socket.close();
  +            }
  +        } catch (Exception e) {
  +            // ignored
           }
  -        //how many bytes to read before updating the timer that data is being transfered
  -        lengthReset = configuration.getChild("lengthReset").getValueAsInteger(20000);
       }
   
       /**
        * @see org.apache.avalon.cornerstone.services.connection.ConnectionHandler#handleConnection(Socket)
        */
       public void handleConnection(Socket connection) throws IOException {
  +
           try {
               this.socket = connection;
               in = new BufferedInputStream(socket.getInputStream(), 1024);
               // An ASCII encoding can be used because all transmissions other
               // that those in the DATA command are guaranteed
               // to be ASCII
  -            inReader = new BufferedReader(new InputStreamReader(in, "ASCII"));
  -            out = new InternetPrintWriter(socket.getOutputStream(), true);
  +            inReader = new BufferedReader(new InputStreamReader(in, "ASCII"), 512);
               remoteIP = socket.getInetAddress().getHostAddress();
               remoteHost = socket.getInetAddress().getHostName();
               smtpID = random.nextInt(1024) + "";
  @@ -322,25 +308,26 @@
           }
   
           try {
  +
  +            out = new InternetPrintWriter(new BufferedWriter(new OutputStreamWriter(socket.getOutputStream()), 1024), false);
  +
               // Initially greet the connector
               // Format is:  Sat, 24 Jan 1998 13:16:09 -0500
   
  -            final PeriodicTimeTrigger trigger = new PeriodicTimeTrigger( timeout, -1 );
  -            scheduler.addTrigger( this.toString(), trigger, this );
  -            StringBuffer responseBuffer =
  -                new StringBuffer(192)
  -                    .append("220 ")
  -                    .append(this.helloName)
  -                    .append(" SMTP Server (")
  -                    .append(SOFTWARE_TYPE)
  -                    .append(") ready ")
  -                    .append(rfc822DateFormat.format(new Date()));
  -            String responseString = responseBuffer.toString();
  +            responseBuffer.append("220 ")
  +                          .append(theConfigData.getHelloName())
  +                          .append(" SMTP Server (")
  +                          .append(SOFTWARE_TYPE)
  +                          .append(") ready ")
  +                          .append(rfc822DateFormat.format(new Date()));
  +            String responseString = clearResponseBuffer();
               writeLoggedFlushedResponse(responseString);
   
  +            theWatchdog.start();
               while (parseCommand(readCommandLine())) {
  -                scheduler.resetTrigger(this.toString());
  +                theWatchdog.reset();
               }
  +            theWatchdog.stop();
               getLogger().debug("Closing socket.");
           } catch (SocketException se) {
               if (getLogger().isDebugEnabled()) {
  @@ -382,38 +369,56 @@
                                      + e.getMessage(), e );
               }
           } finally {
  -            try {
  -                socket.close();
  -            } catch (IOException e) {
  -                if (getLogger().isErrorEnabled()) {
  -                    getLogger().error("Exception closing socket: "
  -                                      + e.getMessage());
  -                }
  -            }
  -            // release from scheduler.
  -            try {
  -                scheduler.removeTrigger(this.toString());
  -            } catch(Throwable t) { }
  +            resetHandler();
           }
       }
   
       /**
  -     * Callback method called when the the PeriodicTimeTrigger in 
  -     * handleConnection is triggered.  In this case the trigger is
  -     * being used as a timeout, so the method simply closes the connection.
  -     *
  -     * @param triggerName the name of the trigger
  -     *
  -     * TODO: Remove this interface from the class and change the mechanism
  +     * Resets the handler data to a basic state.
        */
  -    public void targetTriggered(final String triggerName) {
  -        getLogger().error("Connection timeout on socket with " + remoteHost + " (" + remoteIP + ")");
  +    private void resetHandler() {
  +        resetState();
  +
  +        clearResponseBuffer();
  +        in = null;
  +        inReader = null;
  +        out = null;
  +        remoteHost = null;
  +        remoteIP = null;
  +        authenticatedUser = null;
  +        smtpID = null;
  +
  +        if (theWatchdog != null) {
  +            if (theWatchdog instanceof Disposable) {
  +                ((Disposable)theWatchdog).dispose();
  +            }
  +            theWatchdog = null;
  +        }
  +
           try {
  -            out.println("Connection timeout. Closing connection");
  -            socket.close();
  +            if (socket != null) {
  +                socket.close();
  +            }
           } catch (IOException e) {
  -            // Ignored
  +            if (getLogger().isErrorEnabled()) {
  +                getLogger().error("Exception closing socket: "
  +                                  + e.getMessage());
  +            }
  +        } finally {
  +            socket = null;
           }
  +
  +    }
  +
  +    /**
  +     * Clears the response buffer, returning the String of characters in the buffer.
  +     *
  +     * @return the data in the response buffer
  +     */
  +    private String clearResponseBuffer() {
  +        String responseString = responseBuffer.toString();
  +        responseBuffer.delete(0,responseBuffer.length());
  +        return responseString;
       }
   
       /**
  @@ -488,6 +493,10 @@
        *
        */
       private void resetState() {
  +        ArrayList recipients = (ArrayList)state.get(RCPT_LIST);
  +        if (recipients != null) {
  +            recipients.clear();
  +        }
           state.clear();
       }
   
  @@ -568,14 +577,13 @@
           } else {
               resetState();
               state.put(CURRENT_HELO_MODE, COMMAND_HELO);
  -            StringBuffer responseBuffer = new StringBuffer(256);
  -            if (authRequired) {
  +            if (theConfigData.isAuthRequired()) {
                   //This is necessary because we're going to do a multiline response
                   responseBuffer.append("250-");
               } else {
                   responseBuffer.append("250 ");
               }
  -            responseBuffer.append(helloName)
  +            responseBuffer.append(theConfigData.getHelloName())
                             .append(" Hello ")
                             .append(argument)
                             .append(" (")
  @@ -583,16 +591,13 @@
                             .append(" [")
                             .append(remoteIP)
                             .append("])");
  -            responseString = responseBuffer.toString();
  -            out.println(responseString);
  -            if (authRequired) {
  -                logResponseString(responseString);
  +            responseString = clearResponseBuffer();
  +            if (theConfigData.isAuthRequired()) {
  +                writeLoggedResponse(responseString);
                   responseString = "250 AUTH LOGIN PLAIN";
  -                out.println(responseString);
               }
           }
  -        out.flush();
  -        logResponseString(responseString);
  +            writeLoggedFlushedResponse(responseString);
       }
   
       /**
  @@ -611,18 +616,18 @@
               resetState();
               state.put(CURRENT_HELO_MODE, COMMAND_EHLO);
               // Extension defined in RFC 1870
  -            if (maxmessagesize > 0) {
  -                responseString = "250-SIZE " + maxmessagesize;
  +            long maxMessageSize = theConfigData.getMaxMessageSize();
  +            if (maxMessageSize > 0) {
  +                responseString = "250-SIZE " + maxMessageSize;
                   writeLoggedResponse(responseString);
               }
  -            StringBuffer responseBuffer = new StringBuffer(256);
  -            if (authRequired) {
  +            if (theConfigData.isAuthRequired()) {
                   //This is necessary because we're going to do a multiline response
                   responseBuffer.append("250-");
               } else {
                   responseBuffer.append("250 ");
               }
  -            responseBuffer.append(helloName)
  +            responseBuffer.append(theConfigData.getHelloName())
                              .append(" Hello ")
                              .append(argument)
                              .append(" (")
  @@ -630,8 +635,8 @@
                              .append(" [")
                              .append(remoteIP)
                              .append("])");
  -            responseString = responseBuffer.toString();
  -            if (authRequired) {
  +            responseString = clearResponseBuffer();
  +            if (theConfigData.isAuthRequired()) {
                   writeLoggedResponse(responseString);
                   responseString = "250 AUTH LOGIN PLAIN";
               }
  @@ -698,6 +703,7 @@
                   StringTokenizer authTokenizer = new StringTokenizer(userpass, "\0");
                   user = authTokenizer.nextToken();
                   pass = authTokenizer.nextToken();
  +                authTokenizer = null;
               }
           }
           catch (Exception e) {
  @@ -708,7 +714,7 @@
           if ((user == null) || (pass == null)) {
               responseString = "501 Could not decode parameters for AUTH PLAIN";
               writeLoggedFlushedResponse(responseString);
  -        } else if (users.test(user, pass)) {
  +        } else if (theConfigData.getUsersRepository().test(user, pass)) {
               setUser(user);
               responseString = "235 Authentication Successful";
               writeLoggedFlushedResponse(responseString);
  @@ -760,7 +766,7 @@
           // Authenticate user
           if ((user == null) || (pass == null)) {
               responseString = "501 Could not decode parameters for AUTH LOGIN";
  -        } else if (users.test(user, pass)) {
  +        } else if (theConfigData.getUsersRepository().test(user, pass)) {
               setUser(user);
               responseString = "235 Authentication Successful";
               if (getLogger().isDebugEnabled()) {
  @@ -900,12 +906,10 @@
                   }
               }
               state.put(SENDER, senderAddress);
  -            StringBuffer responseBuffer = 
  -                new StringBuffer(128)
  -                        .append("250 Sender <")
  -                        .append(sender)
  -                        .append("> OK");
  -            responseString = responseBuffer.toString();
  +            responseBuffer.append("250 Sender <")
  +                          .append(sender)
  +                          .append("> OK");
  +            responseString = clearResponseBuffer();
               writeLoggedFlushedResponse(responseString);
           }
       }
  @@ -935,7 +939,8 @@
                       .append(".");
                       getLogger().debug(debugBuffer.toString());
           }
  -        if ((maxmessagesize > 0) && (size > maxmessagesize)) {
  +        long maxMessageSize = theConfigData.getMaxMessageSize();
  +        if ((maxMessageSize > 0) && (size > maxMessageSize)) {
               // Let the client know that the size limit has been hit.
               String responseString = "552 Message size exceeds fixed maximum message size";
               writeLoggedFlushedResponse(responseString);
  @@ -950,7 +955,7 @@
                       .append(") of size ")
                       .append(size)
                       .append(" exceeding system maximum message size of ")
  -                    .append(maxmessagesize)
  +                    .append(maxMessageSize)
                       .append("based on SIZE option.");
               getLogger().error(errorBuffer.toString());
               return false;
  @@ -962,7 +967,6 @@
           return true;
       }
   
  -
       /**
        * Handler method called upon receipt of a RCPT command.
        * Reads recipient.  Does some connection validation.
  @@ -986,9 +990,9 @@
               responseString = "501 Usage: RCPT TO:<recipient>";
               writeLoggedFlushedResponse(responseString);
           } else {
  -            Collection rcptColl = (Collection) state.get(RCPT_VECTOR);
  +            Collection rcptColl = (Collection) state.get(RCPT_LIST);
               if (rcptColl == null) {
  -                rcptColl = new Vector();
  +                rcptColl = new ArrayList();
               }
               recipient = recipient.trim();
               int lastChar = recipient.lastIndexOf('>');
  @@ -1021,6 +1025,7 @@
                           getLogger().debug(debugBuffer.toString());
                       }
                   }
  +                optionTokenizer = null;
               }
               if (!recipient.startsWith("<") || !recipient.endsWith(">")) {
                   responseString = "501 Syntax error in parameters or arguments";
  @@ -1058,12 +1063,12 @@
                   }
                   return;
               }
  -            if (authRequired) {
  +            if (theConfigData.isAuthRequired()) {
                   // Make sure the mail is being sent locally if not
                   // authenticated else reject.
                   if (getUser() == null) {
                       String toDomain = recipientAddress.getHost();
  -                    if (!mailServer.isLocalServer(toDomain)) {
  +                    if (!theConfigData.getMailServer().isLocalServer(toDomain)) {
                           responseString = "530 Authentication Required";
                           writeLoggedFlushedResponse(responseString);
                           getLogger().error("Rejected message - authentication is required for mail request");
  @@ -1071,13 +1076,13 @@
                       }
                   } else {
                       // Identity verification checking
  -                    if (verifyIdentity) {
  +                    if (theConfigData.isVerifyIdentity()) {
                           String authUser = (getUser()).toLowerCase(Locale.US);
                           MailAddress senderAddress = (MailAddress) state.get(SENDER);
                           boolean domainExists = false;
   
                           if ((!authUser.equals(senderAddress.getUser())) ||
  -                            (!mailServer.isLocalServer(senderAddress.getHost()))) {
  +                            (!theConfigData.getMailServer().isLocalServer(senderAddress.getHost()))) {
                               responseString = "503 Incorrect Authentication for Specified Email Address";
                               writeLoggedFlushedResponse(responseString);
                               if (getLogger().isErrorEnabled()) {
  @@ -1095,13 +1100,11 @@
                   }
               }
               rcptColl.add(recipientAddress);
  -            state.put(RCPT_VECTOR, rcptColl);
  -            StringBuffer responseBuffer = 
  -                new StringBuffer(96)
  -                        .append("250 Recipient <")
  -                        .append(recipient)
  -                        .append("> OK");
  -            responseString = responseBuffer.toString();
  +            state.put(RCPT_LIST, rcptColl);
  +            responseBuffer.append("250 Recipient <")
  +                          .append(recipient)
  +                          .append("> OK");
  +            responseString = clearResponseBuffer();
               writeLoggedFlushedResponse(responseString);
           }
       }
  @@ -1126,8 +1129,8 @@
       private void doRSET(String argument) {
           String responseString = "";
           if ((argument == null) || (argument.length() == 0)) {
  -            resetState();
               responseString = "250 OK";
  +            resetState();
           } else {
               responseString = "500 Unexpected argument provided with RSET command";
           }
  @@ -1150,125 +1153,39 @@
           if (!state.containsKey(SENDER)) {
               responseString = "503 No sender specified";
               writeLoggedFlushedResponse(responseString);
  -        } else if (!state.containsKey(RCPT_VECTOR)) {
  +        } else if (!state.containsKey(RCPT_LIST)) {
               responseString = "503 No recipients specified";
               writeLoggedFlushedResponse(responseString);
           } else {
               responseString = "354 Ok Send data ending with <CRLF>.<CRLF>";
               writeLoggedFlushedResponse(responseString);
  +            InputStream msgIn = new CharTerminatedInputStream(in, SMTPTerminator);
               try {
  -                // Setup the input stream to notify the scheduler periodically
  -                InputStream msgIn =
  -                    new SchedulerNotifyInputStream(in, scheduler, this.toString(), 20000);
  -                // Look for data termination
  -                msgIn = new CharTerminatedInputStream(msgIn, SMTPTerminator);
  +                msgIn = new BytesReadResetInputStream(msgIn,
  +                                                      theWatchdog,
  +                                                      theConfigData.getResetLength());
  +
                   // if the message size limit has been set, we'll
                   // wrap msgIn with a SizeLimitedInputStream
  -                if (maxmessagesize > 0) {
  +                long maxMessageSize = theConfigData.getMaxMessageSize();
  +                if (maxMessageSize > 0) {
                       if (getLogger().isDebugEnabled()) {
                           StringBuffer logBuffer = 
                               new StringBuffer(128)
                                       .append("Using SizeLimitedInputStream ")
                                       .append(" with max message size: ")
  -                                    .append(maxmessagesize);
  +                                    .append(maxMessageSize);
                           getLogger().debug(logBuffer.toString());
                       }
  -                    msgIn = new SizeLimitedInputStream(msgIn, maxmessagesize);
  +                    msgIn = new SizeLimitedInputStream(msgIn, maxMessageSize);
                   }
                   // Removes the dot stuffing
                   msgIn = new SMTPInputStream(msgIn);
                   // Parse out the message headers
                   MailHeaders headers = new MailHeaders(msgIn);
  -                // If headers do not contains minimum REQUIRED headers fields,
  -                // add them
  -                if (!headers.isSet(RFC2822Headers.DATE)) {
  -                    headers.setHeader(RFC2822Headers.DATE, rfc822DateFormat.format(new Date()));
  -                }
  -                if (!headers.isSet(RFC2822Headers.FROM) && state.get(SENDER) != null) {
  -                    headers.setHeader(RFC2822Headers.FROM, state.get(SENDER).toString());
  -                }
  -                // Determine the Return-Path
  -                String returnPath = headers.getHeader(RFC2822Headers.RETURN_PATH, "\r\n");
  -                headers.removeHeader(RFC2822Headers.RETURN_PATH);
  -                if (returnPath == null) {
  -                    if (state.get(SENDER) == null) {
  -                        returnPath = "<>";
  -                    } else {
  -                        StringBuffer returnPathBuffer = 
  -                            new StringBuffer(64)
  -                                    .append("<")
  -                                    .append(state.get(SENDER))
  -                                    .append(">");
  -                        returnPath = returnPathBuffer.toString();
  -                    }
  -                }
  -                // We will rebuild the header object to put Return-Path and our
  -                // Received header at the top
  -                Enumeration headerLines = headers.getAllHeaderLines();
  -                headers = new MailHeaders();
  -                // Put the Return-Path first
  -                headers.addHeaderLine(RFC2822Headers.RETURN_PATH + ": " + returnPath);
  -                // Put our Received header next
  -                StringBuffer headerLineBuffer = 
  -                    new StringBuffer(128)
  -                            .append(RFC2822Headers.RECEIVED + ": from ")
  -                            .append(remoteHost)
  -                            .append(" ([")
  -                            .append(remoteIP)
  -                            .append("])");
  -
  -                headers.addHeaderLine(headerLineBuffer.toString());
  -
  -                headerLineBuffer = 
  -                    new StringBuffer(256)
  -                            .append("          by ")
  -                            .append(this.helloName)
  -                            .append(" (")
  -                            .append(SOFTWARE_TYPE)
  -                            .append(") with SMTP ID ")
  -                            .append(smtpID);
  -
  -                if (((Collection) state.get(RCPT_VECTOR)).size() == 1) {
  -                    // Only indicate a recipient if they're the only recipient
  -                    // (prevents email address harvesting and large headers in
  -                    //  bulk email)
  -                    headers.addHeaderLine(headerLineBuffer.toString());
  -                    headerLineBuffer = 
  -                        new StringBuffer(256)
  -                                .append("          for <")
  -                                .append(((Vector) state.get(RCPT_VECTOR)).get(0).toString())
  -                                .append(">;");
  -                    headers.addHeaderLine(headerLineBuffer.toString());
  -                } else {
  -                    // Put the ; on the end of the 'by' line
  -                    headerLineBuffer.append(";");
  -                    headers.addHeaderLine(headerLineBuffer.toString());
  -                }
  -                headers.addHeaderLine("          " + rfc822DateFormat.format(new Date()));
  -
  -                // Add all the original message headers back in next
  -                while (headerLines.hasMoreElements()) {
  -                    headers.addHeaderLine((String) headerLines.nextElement());
  -                }
  -                ByteArrayInputStream headersIn = new ByteArrayInputStream(headers.toByteArray());
  -                MailImpl mail =
  -                    new MailImpl(
  -                        mailServer.getId(),
  -                        (MailAddress) state.get(SENDER),
  -                        (Vector) state.get(RCPT_VECTOR),
  -                        new SequenceInputStream(headersIn, msgIn));
  -                // If the message size limit has been set, we'll
  -                // call mail.getSize() to force the message to be
  -                // loaded. Need to do this to enforce the size limit
  -                if (maxmessagesize > 0) {
  -                    mail.getMessageSize();
  -                }
  -                mail.setRemoteHost(remoteHost);
  -                mail.setRemoteAddr(remoteIP);
  -                mailServer.sendMail(mail);
  -                if (getLogger().isDebugEnabled()) {
  -                    getLogger().debug("Successfully sent mail to spool: " + mail.getName());
  -                }
  +                headers = processMailHeaders(headers);
  +                processMail(headers, msgIn);
  +                headers = null;
               } catch (MessagingException me) {
                   // Grab any exception attached to this one.
                   Exception e = me.getNextException();
  @@ -1293,7 +1210,7 @@
                               .append(" (")
                               .append(remoteIP)
                               .append(") exceeding system maximum message size of ")
  -                            .append(maxmessagesize);
  +                            .append(theConfigData.getMaxMessageSize());
                       getLogger().error(errorBuffer.toString());
                   } else {
                       responseString = "451 Error processing message: "
  @@ -1302,6 +1219,15 @@
                   }
                   writeLoggedFlushedResponse(responseString);
                   return;
  +            } finally {
  +                if (msgIn != null) {
  +                    try {
  +                        msgIn.close();
  +                    } catch (Exception e) {
  +                        // Ignore close exception
  +                    }
  +                    msgIn = null;
  +                }
               }
               resetState();
               responseString = "250 Message received";
  @@ -1309,6 +1235,174 @@
           }
       }
   
  +    private MailHeaders processMailHeaders(MailHeaders headers)
  +        throws MessagingException {
  +        // If headers do not contains minimum REQUIRED headers fields,
  +        // add them
  +        if (!headers.isSet(RFC2822Headers.DATE)) {
  +            headers.setHeader(RFC2822Headers.DATE, rfc822DateFormat.format(new Date()));
  +        }
  +        if (!headers.isSet(RFC2822Headers.FROM) && state.get(SENDER) != null) {
  +            headers.setHeader(RFC2822Headers.FROM, state.get(SENDER).toString());
  +        }
  +        // Determine the Return-Path
  +        String returnPath = headers.getHeader(RFC2822Headers.RETURN_PATH, "\r\n");
  +        headers.removeHeader(RFC2822Headers.RETURN_PATH);
  +        StringBuffer headerLineBuffer = new StringBuffer(512);
  +        if (returnPath == null) {
  +            if (state.get(SENDER) == null) {
  +                returnPath = "<>";
  +            } else {
  +                headerLineBuffer.append("<")
  +                                .append(state.get(SENDER))
  +                                .append(">");
  +                returnPath = headerLineBuffer.toString();
  +                headerLineBuffer.delete(0, headerLineBuffer.length());
  +            }
  +        }
  +        // We will rebuild the header object to put Return-Path and our
  +        // Received header at the top
  +        Enumeration headerLines = headers.getAllHeaderLines();
  +        MailHeaders newHeaders = new MailHeaders();
  +        // Put the Return-Path first
  +        newHeaders.addHeaderLine(RFC2822Headers.RETURN_PATH + ": " + returnPath);
  +        // Put our Received header next
  +        headerLineBuffer.append(RFC2822Headers.RECEIVED + ": from ")
  +                        .append(remoteHost)
  +                        .append(" ([")
  +                        .append(remoteIP)
  +                        .append("])");
  +
  +        newHeaders.addHeaderLine(headerLineBuffer.toString());
  +        headerLineBuffer.delete(0, headerLineBuffer.length());
  +
  +        headerLineBuffer.append("          by ")
  +                        .append(theConfigData.getHelloName())
  +                        .append(" (")
  +                        .append(SOFTWARE_TYPE)
  +                        .append(") with SMTP ID ")
  +                        .append(smtpID);
  +
  +        if (((Collection) state.get(RCPT_LIST)).size() == 1) {
  +            // Only indicate a recipient if they're the only recipient
  +            // (prevents email address harvesting and large headers in
  +            //  bulk email)
  +            newHeaders.addHeaderLine(headerLineBuffer.toString());
  +            headerLineBuffer.delete(0, headerLineBuffer.length());
  +            headerLineBuffer.append("          for <")
  +                            .append(((List) state.get(RCPT_LIST)).get(0).toString())
  +                            .append(">;");
  +            newHeaders.addHeaderLine(headerLineBuffer.toString());
  +            headerLineBuffer.delete(0, headerLineBuffer.length());
  +        } else {
  +            // Put the ; on the end of the 'by' line
  +            headerLineBuffer.append(";");
  +            newHeaders.addHeaderLine(headerLineBuffer.toString());
  +            headerLineBuffer.delete(0, headerLineBuffer.length());
  +        }
  +        headerLineBuffer = null;
  +        newHeaders.addHeaderLine("          " + rfc822DateFormat.format(new Date()));
  +
  +        // Add all the original message headers back in next
  +        while (headerLines.hasMoreElements()) {
  +            newHeaders.addHeaderLine((String) headerLines.nextElement());
  +        }
  +        return newHeaders;
  +    }
  +
  +    /**
  +     * Processes the mail message coming in off the wire.  Reads the
  +     * content and delivers to the spool.
  +     *
  +     * @param headers the headers of the mail being read
  +     * @param msgIn the stream containing the message content
  +     */
  +    private void processMail(MailHeaders headers, InputStream msgIn)
  +        throws MessagingException {
  +        ByteArrayInputStream headersIn = null;
  +        MailImpl mail = null;
  +        List recipientCollection = null;
  +        try {
  +            headersIn = new ByteArrayInputStream(headers.toByteArray());
  +            recipientCollection = (List) state.get(RCPT_LIST);
  +            mail =
  +                new MailImpl(theConfigData.getMailServer().getId(),
  +                             (MailAddress) state.get(SENDER),
  +                             recipientCollection,
  +                             new SequenceInputStream(headersIn, msgIn));
  +            // Call mail.getSize() to force the message to be
  +            // loaded. Need to do this to enforce the size limit
  +            if (theConfigData.getMaxMessageSize() > 0) {
  +                mail.getMessageSize();
  +            }
  +            mail.setRemoteHost(remoteHost);
  +            mail.setRemoteAddr(remoteIP);
  +            theConfigData.getMailServer().sendMail(mail);
  +            Collection theRecipients = mail.getRecipients();
  +            String recipientString = "";
  +            if (theRecipients != null) {
  +                recipientString = theRecipients.toString();
  +            }
  +            if (getLogger().isDebugEnabled()) {
  +                StringBuffer debugBuffer =
  +                    new StringBuffer(256)
  +                        .append("Successfully spooled mail )")
  +                        .append(mail.getName())
  +                        .append(" from ")
  +                        .append(mail.getSender())
  +                        .append(" for ")
  +                        .append(recipientString);
  +                getLogger().debug(debugBuffer.toString());
  +            } else if (getLogger().isInfoEnabled()) {
  +                StringBuffer infoBuffer =
  +                    new StringBuffer(256)
  +                        .append("Successfully spooled mail from ")
  +                        .append(mail.getSender())
  +                        .append(" for ")
  +                        .append(recipientString);
  +                getLogger().info(infoBuffer.toString());
  +            }
  +        } finally {
  +            if (recipientCollection != null) {
  +                recipientCollection.clear();
  +            }
  +            recipientCollection = null;
  +            if (mail != null) {
  +                mail.dispose();
  +            }
  +            mail = null;
  +            if (headersIn != null) {
  +                try {
  +                    headersIn.close();
  +                } catch (IOException ioe) {
  +                    // Ignore exception on close.
  +                }
  +            }
  +            headersIn = null;
  +        }
  +    }
  +
  +    /**
  +     * Handler method called upon receipt of a QUIT command.
  +     * This method informs the client that the connection is
  +     * closing.
  +     *
  +     * @param argument the argument passed in with the command by the SMTP client
  +     */
  +    private void doQUIT(String argument) {
  +
  +        String responseString = "";
  +        if ((argument == null) || (argument.length() == 0)) {
  +            responseBuffer.append("221 ")
  +                          .append(theConfigData.getHelloName())
  +                          .append(" Service closing transmission channel");
  +            responseString = clearResponseBuffer();
  +        } else {
  +            responseString = "500 Unexpected argument provided with QUIT command";
  +        }
  +        writeLoggedFlushedResponse(responseString);
  +    }
  +
       /**
        * Handler method called upon receipt of a VRFY command.
        * This method informs the client that the command is
  @@ -1317,7 +1411,6 @@
        * @param argument the argument passed in with the command by the SMTP client
        */
       private void doVRFY(String argument) {
  -
           String responseString = "502 VRFY is not supported";
           writeLoggedFlushedResponse(responseString);
       }
  @@ -1349,29 +1442,6 @@
       }
   
       /**
  -     * Handler method called upon receipt of a QUIT command.
  -     * This method informs the client that the connection is
  -     * closing.
  -     *
  -     * @param argument the argument passed in with the command by the SMTP client
  -     */
  -    private void doQUIT(String argument) {
  -
  -        String responseString = "";
  -        if ((argument == null) || (argument.length() == 0)) {
  -            StringBuffer responseBuffer =
  -                new StringBuffer(128)
  -                        .append("221 ")
  -                        .append(helloName)
  -                        .append(" Service closing transmission channel");
  -            responseString = responseBuffer.toString();
  -        } else {
  -            responseString = "500 Unexpected argument provided with QUIT command";
  -        }
  -        writeLoggedFlushedResponse(responseString);
  -    }
  -
  -    /**
        * Handler method called upon receipt of an unrecognized command.
        * Returns an error response and logs the command.
        *
  @@ -1379,13 +1449,29 @@
        * @param argument the argument passed in with the command by the SMTP client
        */
       private void doUnknownCmd(String command, String argument) {
  -        StringBuffer responseBuffer =
  -            new StringBuffer(128)
  -                    .append("500 ")
  -                    .append(helloName)
  -                    .append(" Syntax error, command unrecognized: ")
  -                    .append(command);
  -        String responseString = responseBuffer.toString();
  +        responseBuffer.append("500 ")
  +                      .append(theConfigData.getHelloName())
  +                      .append(" Syntax error, command unrecognized: ")
  +                      .append(command);
  +        String responseString = clearResponseBuffer();
           writeLoggedFlushedResponse(responseString);
       }
  +
  +    /**
  +     * A private inner class which serves as an adaptor
  +     * between the WatchdogTarget interface and this
  +     * handler class.
  +     */
  +    private class SMTPWatchdogTarget
  +        implements WatchdogTarget {
  +
  +        /**
  +         * @see org.apache.james.util.watchdog.WatchdogTarget#execute()
  +         */
  +        public void execute() {
  +            SMTPHandler.this.idleClose();
  +        }
  +
  +    }
  +
   }
  
  
  
  1.14      +260 -52   jakarta-james/src/java/org/apache/james/smtpserver/SMTPServer.java
  
  Index: SMTPServer.java
  ===================================================================
  RCS file: /home/cvs/jakarta-james/src/java/org/apache/james/smtpserver/SMTPServer.java,v
  retrieving revision 1.13
  retrieving revision 1.14
  diff -u -r1.13 -r1.14
  --- SMTPServer.java	2 Oct 2002 06:12:03 -0000	1.13
  +++ SMTPServer.java	26 Oct 2002 04:15:30 -0000	1.14
  @@ -6,16 +6,32 @@
    * the LICENSE file.
    */
   package org.apache.james.smtpserver;
  -import org.apache.avalon.cornerstone.services.connection.AbstractService;
  -import org.apache.avalon.cornerstone.services.connection.ConnectionHandlerFactory;
  -import org.apache.avalon.cornerstone.services.connection.DefaultHandlerFactory;
  +
  +import org.apache.avalon.cornerstone.services.connection.ConnectionHandler;
  +import org.apache.avalon.excalibur.pool.DefaultPool;
  +import org.apache.avalon.excalibur.pool.HardResourceLimitingPool;
  +import org.apache.avalon.excalibur.pool.ObjectFactory;
  +import org.apache.avalon.excalibur.pool.Pool;
  +import org.apache.avalon.excalibur.pool.Poolable;
  +import org.apache.avalon.framework.activity.Disposable;
  +import org.apache.avalon.framework.activity.Initializable;
   import org.apache.avalon.framework.component.ComponentException;
   import org.apache.avalon.framework.component.ComponentManager;
   import org.apache.avalon.framework.configuration.Configuration;
   import org.apache.avalon.framework.configuration.ConfigurationException;
   import org.apache.avalon.framework.component.Component;
  -import org.apache.mailet.MailetContext;
  +import org.apache.avalon.framework.activity.Disposable;
  +import org.apache.avalon.framework.logger.LogEnabled;
   import org.apache.james.Constants;
  +import org.apache.james.core.AbstractJamesService;
  +import org.apache.james.services.MailServer;
  +import org.apache.james.services.UsersRepository;
  +import org.apache.james.services.UsersStore;
  +import org.apache.james.util.watchdog.ThreadPerWatchdogFactory;
  +import org.apache.james.util.watchdog.Watchdog;
  +import org.apache.james.util.watchdog.WatchdogFactory;
  +import org.apache.james.util.watchdog.WatchdogTarget;
  +import org.apache.mailet.MailetContext;
   import java.net.InetAddress;
   import java.net.UnknownHostException;
   /**
  @@ -28,25 +44,77 @@
    * @author  Matthew Pangaro <ma...@lokitech.com>
    * @author  <a href="mailto:donaldp@apache.org">Peter Donald</a>
    * @author  <a href="mailto:danny@apache.org">Danny Angus</a>
  + * @author Peter M. Goldstein <fa...@alum.mit.edu>
    */
   /*
  - * IMPORTANT: SMTPServer extends AbstractService.  If you implement ANY
  + * IMPORTANT: SMTPServer extends AbstractJamesService.  If you implement ANY
    * lifecycle methods, you MUST call super.<method> as well.
    */
  -public class SMTPServer extends AbstractService implements Component {
  +public class SMTPServer extends AbstractJamesService implements Component {
  +
       /**
        * The mailet context - we access it here to set the hello name for the Mailet API
        */
       MailetContext mailetcontext;
   
       /**
  -     * Whether this service is enabled
  +     * The user repository for this server - used to authenticate
  +     * users.
        */
  -    private volatile boolean enabled = true;
  +    private UsersRepository users;
   
  -    protected ConnectionHandlerFactory createFactory() {
  -        return new DefaultHandlerFactory(SMTPHandler.class);
  -    }
  +    /**
  +     * The internal mail server service.
  +     */
  +    private MailServer mailServer;
  +
  +    /**
  +     * Whether authentication is required to use
  +     * this SMTP server.
  +     */
  +    private boolean authRequired = false;
  +
  +    /**
  +     * Whether the server verifies that the user
  +     * actually sending an email matches the
  +     * authentication credentials attached to the
  +     * SMTP interaction.
  +     */
  +    private boolean verifyIdentity = false;
  +
  +    /**
  +     * The maximum message size allowed by this SMTP server.  The default
  +     * value, 0, means no limit.
  +     */
  +    private long maxMessageSize = 0;
  +
  +    /**
  +     * The number of bytes to read before resetting
  +     * the connection timeout timer.  Defaults to
  +     * 20 KB.
  +     */
  +    private int lengthReset = 20 * 1024;
  +
  +    /**
  +     * The pool used to provide SMTP Handler objects
  +     */
  +    private Pool theHandlerPool = null;
  +
  +    /**
  +     * The pool used to provide SMTP Handler objects
  +     */
  +    private ObjectFactory theHandlerFactory = new SMTPHandlerFactory();
  +
  +    /**
  +     * The factory used to generate Watchdog objects
  +     */
  +    private WatchdogFactory theWatchdogFactory;
  +
  +    /**
  +     * The configuration data to be passed to the handler
  +     */
  +    private SMTPHandlerConfigurationData theConfigData
  +        = new SMTPHandlerConfigurationDataImpl();
   
       /**
        * @see org.apache.avalon.framework.component.Composable#compose(ComponentManager)
  @@ -54,31 +122,47 @@
       public void compose(final ComponentManager componentManager) throws ComponentException {
           super.compose(componentManager);
           mailetcontext = (MailetContext) componentManager.lookup("org.apache.mailet.MailetContext");
  +        mailServer = (MailServer) componentManager.lookup("org.apache.james.services.MailServer");
  +        UsersStore usersStore =
  +            (UsersStore) componentManager.lookup("org.apache.james.services.UsersStore");
  +        users = usersStore.getRepository("LocalUsers");
  +        if (users == null) {
  +            throw new ComponentException("The user repository could not be found.");
  +        }
       }
   
       /**
        * @see org.apache.avalon.framework.configuration.Configurable#configure(Configuration)
        */
       public void configure(final Configuration configuration) throws ConfigurationException {
  -        enabled = configuration.getAttributeAsBoolean("enabled", true);
  -        if (enabled) {
  -            m_port = configuration.getChild("port").getValueAsInteger(25);
  -            try {
  -                final String bindAddress = configuration.getChild("bind").getValue(null);
  -                if (null != bindAddress) {
  -                    m_bindTo = InetAddress.getByName(bindAddress);
  +        super.configure(configuration);
  +        if (isEnabled()) {
  +            mailetcontext.setAttribute(Constants.HELLO_NAME, helloName);
  +            Configuration handlerConfiguration = configuration.getChild("handler");
  +            authRequired = handlerConfiguration.getChild("authRequired").getValueAsBoolean(false);
  +            verifyIdentity = handlerConfiguration.getChild("verifyIdentity").getValueAsBoolean(false);
  +            if (authRequired) {
  +                if (verifyIdentity) {
  +                    getLogger().info("This SMTP server requires authentication and verifies that the authentication credentials match the sender address.");
  +                } else {
  +                    getLogger().info("This SMTP server requires authentication, but doesn't verify that the authentication credentials match the sender address.");
                   }
  -            } catch (final UnknownHostException unhe) {
  -                throw new ConfigurationException("Malformed bind parameter", unhe);
  +            } else {
  +                getLogger().info("This SMTP server does not require authentication.");
               }
  -            final boolean useTLS = configuration.getChild("useTLS").getValueAsBoolean(false);
  -            if (useTLS) {
  -                m_serverSocketType = "ssl";
  +            // get the message size limit from the conf file and multiply
  +            // by 1024, to put it in bytes
  +            maxMessageSize = handlerConfiguration.getChild( "maxmessagesize" ).getValueAsLong( maxMessageSize ) * 1024;
  +            if (getLogger().isInfoEnabled()) {
  +                getLogger().info("The maximum allowed message size is " + maxMessageSize + " bytes.");
               }
  -            super.configure(configuration.getChild("handler"));
  -            // make our "helloName" available through the MailetContext
  -            String helloName = SMTPHandler.configHelloName(configuration.getChild("handler"));
  -            mailetcontext.setAttribute(Constants.HELLO_NAME, helloName);
  +            //how many bytes to read before updating the timer that data is being transfered
  +            lengthReset = configuration.getChild("lengthReset").getValueAsInteger(lengthReset);
  +            if (getLogger().isInfoEnabled()) {
  +                getLogger().info("The idle timeout will be reset every " + lengthReset + " bytes.");
  +            }
  +        } else {
  +            mailetcontext.setAttribute(Constants.HELLO_NAME, "localhost");
           }
       }
   
  @@ -86,37 +170,161 @@
        * @see org.apache.avalon.framework.activity.Initializable#initialize()
        */
       public void initialize() throws Exception {
  -        if (enabled) {
  -            getLogger().info("SMTPServer init...");
  -            super.initialize();
  -            StringBuffer logBuffer =
  -                new StringBuffer(128)
  -                    .append("SMTPServer using ")
  -                    .append(m_serverSocketType)
  -                    .append(" on port ")
  -                    .append(m_port)
  -                    .append(" at ")
  -                    .append(m_bindTo);
  -            getLogger().info(logBuffer.toString());
  -            getLogger().info("SMTPServer ...init end");
  -            System.out.println("SMTP Server Started " + m_connectionName);
  +        super.initialize();
  +        if (!isEnabled()) {
  +            return;
  +        }
  +
  +        if (connectionLimit != null) {
  +            theHandlerPool = new HardResourceLimitingPool(theHandlerFactory, 5, connectionLimit.intValue());
  +            if (getLogger().isDebugEnabled()) {
  +                getLogger().debug("Using a bounded pool for SMTP handlers with upper limit " + connectionLimit.intValue());
  +            }
           } else {
  -            getLogger().info("SMTP Server Disabled");
  -            System.out.println("SMTP Server Disabled");
  +            // NOTE: The maximum here is not a real maximum.  The handler pool will continue to
  +            //       provide handlers beyond this value.
  +            theHandlerPool = new DefaultPool(theHandlerFactory, null, 5, 30);
  +            getLogger().debug("Using an unbounded pool for SMTP handlers.");
  +        }
  +        if (theHandlerPool instanceof LogEnabled) {
  +            ((LogEnabled)theHandlerPool).enableLogging(getLogger());
  +        }
  +        if (theHandlerPool instanceof Initializable) {
  +            ((Initializable)theHandlerPool).initialize();
  +        }
  +
  +        theWatchdogFactory = new ThreadPerWatchdogFactory(threadPool, timeout);
  +        if (theWatchdogFactory instanceof LogEnabled) {
  +            ((LogEnabled)theWatchdogFactory).enableLogging(getLogger());
           }
       }
   
       /**
  -     * @see org.apache.avalon.framework.activity.Disposable#dispose()
  +     * @see org.apache.james.core.AbstractJamesService#getDefaultPort()
        */
  -    public void dispose() {
  -        if (enabled) {
  -            getLogger().info("SMTPServer dispose...");
  -            getLogger().info("SMTPServer dispose..." + m_connectionName);
  -            super.dispose();
  -            // This is needed to make sure sockets are promptly closed on Windows 2000
  -            System.gc();
  -            getLogger().info("SMTPServer ...dispose end");
  +     protected int getDefaultPort() {
  +        return 25;
  +     }
  +
  +    /**
  +     * @see org.apache.james.core.AbstractJamesService#getServiceType()
  +     */
  +    public String getServiceType() {
  +        return "SMTP Service - Watchdogs unpooled, buffers adjusted with sendMail disabled";
  +    }
  +
  +    /**
  +     * @see org.apache.avalon.cornerstone.services.connection.AbstractHandlerFactory#newHandler()
  +     */
  +    protected ConnectionHandler newHandler()
  +            throws Exception {
  +        SMTPHandler theHandler = (SMTPHandler)theHandlerPool.get();
  +
  +        if (getLogger().isDebugEnabled()) {
  +            getLogger().debug("Getting SMTPHandler from pool.");
  +        }
  +        Watchdog theWatchdog = theWatchdogFactory.getWatchdog(theHandler.getWatchdogTarget());
  +
  +        theHandler.setConfigurationData(theConfigData);
  +
  +        theHandler.setWatchdog(theWatchdog);
  +        return theHandler;
  +    }
  +
  +    /**
  +     * @see org.apache.avalon.cornerstone.services.connection.ConnectionHandlerFactory#releaseConnectionHandler(ConnectionHandler)
  +     */
  +    public void releaseConnectionHandler( ConnectionHandler connectionHandler ) {
  +        if (!(connectionHandler instanceof SMTPHandler)) {
  +            throw new IllegalArgumentException("Attempted to return non-SMTPHandler to pool.");
  +        }
  +        if (getLogger().isDebugEnabled()) {
  +            getLogger().debug("Returning SMTPHandler to pool.");
  +        }
  +        theHandlerPool.put((Poolable)connectionHandler);
  +    }
  +
  +    /**
  +     * The factory for producing handlers.
  +     */
  +    private static class SMTPHandlerFactory
  +        implements ObjectFactory {
  +
  +        /**
  +         * @see org.apache.avalon.excalibur.pool.ObjectFactory#newInstance()
  +         */
  +        public Object newInstance() throws Exception {
  +            return new SMTPHandler();
  +        }
  +
  +        /**
  +         * @see org.apache.avalon.excalibur.pool.ObjectFactory#getCreatedClass()
  +         */
  +        public Class getCreatedClass() {
  +            return SMTPHandler.class;
  +        }
  +
  +        /**
  +         * @see org.apache.avalon.excalibur.pool.ObjectFactory#decommision(Object)
  +         */
  +        public void decommission( Object object ) throws Exception {
  +            return;
  +        }
  +    }
  +
  +    /**
  +     * A class to provide SMTP handler configuration to the handlers
  +     */
  +    private class SMTPHandlerConfigurationDataImpl
  +        implements SMTPHandlerConfigurationData {
  +
  +        /**
  +         * @see org.apache.james.smtpserver.SMTPHandlerConfigurationData#getHelloName()
  +         */
  +        public String getHelloName() {
  +            return SMTPServer.this.helloName;
  +        }
  +
  +        /**
  +         * @see org.apache.james.smtpserver.SMTPHandlerConfigurationData#getResetLength()
  +         */
  +        public int getResetLength() {
  +            return SMTPServer.this.lengthReset;
  +        }
  +
  +        /**
  +         * @see org.apache.james.smtpserver.SMTPHandlerConfigurationData#getMaxMessageSize()
  +         */
  +        public long getMaxMessageSize() {
  +            return SMTPServer.this.maxMessageSize;
  +        }
  +
  +        /**
  +         * @see org.apache.james.smtpserver.SMTPHandlerConfigurationData#isAuthRequired()
  +         */
  +        public boolean isAuthRequired() {
  +            return SMTPServer.this.authRequired;
  +        }
  +
  +        /**
  +         * @see org.apache.james.smtpserver.SMTPHandlerConfigurationData#isVerifyIdentity()
  +         */
  +        public boolean isVerifyIdentity() {
  +            return SMTPServer.this.verifyIdentity;
  +        }
  +
  +        /**
  +         * @see org.apache.james.smtpserver.SMTPHandlerConfigurationData#getMailServer()
  +         */
  +        public MailServer getMailServer() {
  +            return SMTPServer.this.mailServer;
  +        }
  +
  +        /**
  +         * @see org.apache.james.smtpserver.SMTPHandlerConfigurationData#getUsersRepository()
  +         */
  +        public UsersRepository getUsersRepository() {
  +            return SMTPServer.this.users;
           }
       }
   }
  
  
  
  1.4       +3 -3      jakarta-james/src/java/org/apache/james/smtpserver/SMTPServer.xinfo
  
  Index: SMTPServer.xinfo
  ===================================================================
  RCS file: /home/cvs/jakarta-james/src/java/org/apache/james/smtpserver/SMTPServer.xinfo,v
  retrieving revision 1.3
  retrieving revision 1.4
  diff -u -r1.3 -r1.4
  --- SMTPServer.xinfo	23 Aug 2002 08:53:36 -0000	1.3
  +++ SMTPServer.xinfo	26 Oct 2002 04:15:30 -0000	1.4
  @@ -30,10 +30,10 @@
         <service name="org.apache.avalon.cornerstone.services.sockets.SocketManager" version="1.0"/>
       </dependency>
       <dependency>
  -      <service name="org.apache.avalon.cornerstone.services.scheduler.TimeScheduler" version="1.0"/>
  -    </dependency> 
  -    <dependency>
         <service name="org.apache.james.services.MailServer" version="1.0"/>
       </dependency> 
  +    <dependency>
  +      <service name="org.apache.avalon.cornerstone.services.threads.ThreadManager" version="1.0"/>
  +    </dependency>
     </dependencies>  
   </blockinfo>
  
  
  
  1.1                  jakarta-james/src/java/org/apache/james/smtpserver/SMTPHandlerConfigurationData.java
  
  Index: SMTPHandlerConfigurationData.java
  ===================================================================
  /*
   * Copyright (C) The Apache Software Foundation. All rights reserved.
   *
   * This software is published under the terms of the Apache Software License
   * version 1.1, a copy of which has been included with this distribution in
   * the LICENSE file.
   */
  package org.apache.james.smtpserver;
  
  import org.apache.james.services.MailServer;
  import org.apache.james.services.UsersRepository;
  
  /**
   * Provides a number of server-wide constant values to the
   * SMTPHandlers
   *
   * @author Peter M. Goldstein <fa...@alum.mit.edu>
   */
  public interface SMTPHandlerConfigurationData {
  
      /**
       * Returns the service wide hello name
       *
       * @return the hello name
       */
      String getHelloName();
  
      /**
       * Returns the service wide reset length in bytes.
       *
       * @return the reset length
       */
      int getResetLength();
  
      /**
       * Returns the service wide maximum message size in bytes.
       *
       * @return the maximum message size
       */
      long getMaxMessageSize();
  
      /**
       * Returns whether SMTP auth is active for this server.
       *
       * @return whether SMTP authentication is on
       */
      boolean isAuthRequired();
  
      /**
       * Returns whether the service validates the identity
       * of its senders.
       *
       * @return whether SMTP authentication is on
       */
      boolean isVerifyIdentity();
  
      /**
       * Returns the MailServer interface for this service.
       *
       * @return the MailServer interface for this service
       */
      MailServer getMailServer();
  
      /**
       * Returns the UsersRepository for this service.
       *
       * @return the local users repository
       */
      UsersRepository getUsersRepository();
  
  }
  
  
  
  1.1                  jakarta-james/src/java/org/apache/james/util/watchdog/BytesReadResetInputStream.java
  
  Index: BytesReadResetInputStream.java
  ===================================================================
  /*
   * Copyright (C) The Apache Software Foundation. All rights reserved.
   *
   * This software is published under the terms of the Apache Software License
   * version 1.1, a copy of which has been included with this distribution in
   * the LICENSE file.
   */
  
  package org.apache.james.util.watchdog;
  
  import java.io.IOException;
  import java.io.InputStream;
  
  /**
   * This will reset the Watchdog each time a certain amount of data has
   * been transferred.  This allows us to keep the timeout settings low, while
   * not timing out during large data transfers.
   */
  public class BytesReadResetInputStream extends InputStream {
  
      /**
       * The wrapped InputStream
       */
      private InputStream in = null;
  
      /**
       * The Watchdog to be reset every lengthReset bytes
       */
      private Watchdog watchdog;
  
      /**
       * The number of bytes that need to be read before the counter is reset.
       */
      private int lengthReset = 0;
  
      /**
       * The number of bytes read since the counter was last reset
       */
      int readCounter = 0;
  
      /**
       * @param in the InputStream to be wrapped by this stream
       * @param watchdog the watchdog to be reset
       * @param lengthReset the number of bytes to be read in between trigger resets
       */
      public BytesReadResetInputStream(InputStream in,
                                       Watchdog watchdog, 
                                       int lengthReset) {
          this.in = in;
          this.watchdog = watchdog;
          this.lengthReset = lengthReset;
  
          readCounter = 0;
      }
  
      /**
       * Read an array of bytes from the stream
       *
       * @param b the array of bytes to read from the stream
       * @param off the index in the array where we start writing
       * @param len the number of bytes of the array to read
       *
       * @return the number of bytes read
       *
       * @throws IOException if an exception is encountered when reading
       */
      public int read(byte[] b, int off, int len) throws IOException {
          int l = in.read(b, off, len);
          readCounter += l;
  
          if (readCounter > lengthReset) {
              readCounter = 0;
              watchdog.reset();
          }
  
          return l;
      }
  
      /**
       * Read a byte from the stream
       *
       * @return the byte read from the stream
       * @throws IOException if an exception is encountered when reading
       */
      public int read() throws IOException {
          int b = in.read();
          readCounter++;
  
          if (readCounter > lengthReset) {
              readCounter = 0;
              watchdog.reset();
          }
  
          return b;
      }
  
      /**
       * Close the stream
       *
       * @throws IOException if an exception is encountered when closing
       */
      public void close() throws IOException {
          in.close();
      }
  }
  
  
  
  1.1                  jakarta-james/src/java/org/apache/james/util/watchdog/BytesWrittenResetOutputStream.java
  
  Index: BytesWrittenResetOutputStream.java
  ===================================================================
  /*
   * Copyright (C) The Apache Software Foundation. All rights reserved.
   *
   * This software is published under the terms of the Apache Software License
   * version 1.1, a copy of which has been included with this distribution in
   * the LICENSE file.
   */
  
  package org.apache.james.util.watchdog;
  
  import java.io.IOException;
  import java.io.OutputStream;
  
  /**
   * This will reset the Watchdog each time a certain amount of data has
   * been transferred.  This allows us to keep the timeout settings low, while
   * not timing out during large data transfers.
   */
  public class BytesWrittenResetOutputStream extends OutputStream {
  
      /**
       * The output stream wrapped by this method
       */
      OutputStream out = null;
  
      /**
       * The Watchdog to be reset every lengthReset bytes
       */
      private Watchdog watchdog;
  
      /**
       * The number of bytes that need to be written before the counter is reset.
       */
      int lengthReset = 0;
  
      /**
       * The number of bytes written since the counter was last reset
       */
      int writtenCounter = 0;
  
      public BytesWrittenResetOutputStream(OutputStream out,
                                           Watchdog watchdog,
                                           int lengthReset) {
          this.out = out;
          this.watchdog = watchdog;
          this.lengthReset = lengthReset;
  
          writtenCounter = 0;
      }
  
      /**
       * Write an array of bytes to the stream
       *
       * @param b the array of bytes to write to the stream
       * @param off the index in the array where we start writing
       * @param len the number of bytes of the array to write
       *
       * @throws IOException if an exception is encountered when writing
       */
      public void write(byte[] b, int off, int len) throws IOException {
          out.write(b, off, len);
          writtenCounter += len;
  
          if (writtenCounter > lengthReset) {
              writtenCounter = 0;
              watchdog.reset();
          }
      }
  
      /**
       * Write a byte to the stream
       *
       * @param b the byte to write to the stream
       *
       * @throws IOException if an exception is encountered when writing
       */
      public void write(int b) throws IOException {
          out.write(b);
          writtenCounter++;
  
          if (writtenCounter > lengthReset) {
              writtenCounter = 0;
              watchdog.reset();
          }
      }
  
      /**
       * Flush the stream
       *
       * @throws IOException if an exception is encountered when flushing
       */
      public void flush() throws IOException {
          out.flush();
      }
  
      /**
       * Close the stream
       *
       * @throws IOException if an exception is encountered when closing
       */
      public void close() throws IOException {
          out.close();
      }
  }
  
  
  
  1.1                  jakarta-james/src/java/org/apache/james/util/watchdog/InaccurateTimeoutWatchdog.java
  
  Index: InaccurateTimeoutWatchdog.java
  ===================================================================
  /*
   * Copyright (C) The Apache Software Foundation. All rights reserved.
   *
   * This software is published under the terms of the Apache Software License
   * version 1.1, a copy of which has been included  with this distribution in
   * the LICENSE file.
   */
  package org.apache.james.util.watchdog;
  
  import org.apache.avalon.excalibur.thread.ThreadPool;
  import org.apache.avalon.framework.activity.Disposable;
  import org.apache.avalon.framework.logger.AbstractLogEnabled;
  
  /**
   * This class represents an watchdog process that serves to
   * monitor a situation and triggers an action after a certain time has
   * passed.  This implementation is deliberately inaccurate, trading
   * accuracy for minimal impact on reset.  This should be used when
   * the time of the Watchdog trigger is not critical, and a high number
   * of resets are expected.
   *
   * @author Andrei Ivanov
   * @author Peter M. Goldstein <fa...@alum.mit.edu>
   */
  public class InaccurateTimeoutWatchdog
      extends AbstractLogEnabled
      implements Watchdog, Runnable, Disposable {
  
      /**
       * Whether the watchdog is currently checking the trigger condition
       */
      private volatile boolean isChecking = false;
  
      /**
       * Whether the watchdog has been reset since the thread slept.
       */
      private volatile boolean isReset = false;
  
  
      /**
       * The number of milliseconds until the watchdog times out.
       */
      private final long timeout;
  
      /**
       * The last time the internal timer was reset, as measured in milliseconds since
       * January 1, 1970 00:00:00.000 GMT.
       */
      private volatile long lastReset;
  
      /**
       * The WatchdogTarget whose execute() method will be called upon triggering
       * of the condition.
       */
      private WatchdogTarget triggerTarget;
  
      /**
       * The thread that runs the watchdog.
       */
      private Thread watchdogThread;
  
      /**
       * The thread pool used to generate InaccurateTimeoutWatchdogs
       */
      private ThreadPool myThreadPool;
  
      /**
       * The sole constructor for the InaccurateTimeoutWatchdog
       *
       * @param timeout the time (in msec) that it will take the Watchdog to timeout
       * @param target the WatchdogTarget to be executed when this Watchdog expires
       * @param threadPool the thread pool used to generate threads for this implementation.
       */
      public InaccurateTimeoutWatchdog(long timeout, WatchdogTarget target, ThreadPool threadPool) {
          if (target == null) {
              throw new IllegalArgumentException("The WatchdogTarget for this TimeoutWatchdog cannot be null.");
          }
          if (threadPool == null) {
              throw new IllegalArgumentException("The thread pool for this TimeoutWatchdog cannot be null.");
          }
          this.timeout = timeout;
          triggerTarget = target;
          myThreadPool = threadPool;
      }
  
      /**
       * Start this Watchdog, causing it to begin checking.
       */
      public void start() {
          getLogger().debug("Calling start()");
          lastReset = System.currentTimeMillis();
          isChecking = true;
          synchronized(this) {
              if ( watchdogThread == null) {
                  myThreadPool.execute(this);
              }
          }
      }
  
      /**
       * Reset this Watchdog.  Tells the Watchdog thread to reset
       * the timer when it next awakens.
       */
      public void reset() {
          if (watchdogThread != null) {
              getLogger().debug("Calling reset() " + watchdogThread.getName());
          } else {
              getLogger().debug("Calling reset() for inactive watchdog");
          }
          isReset = true;
      }
  
      /**
       * Stop this Watchdog, causing the Watchdog to stop checking the trigger
       * condition.  The monitor can be restarted with a call to startWatchdog.
       */
      public void stop() {
          if (watchdogThread != null) {
              getLogger().debug("Calling stop() " + watchdogThread.getName());
          } else {
              getLogger().debug("Calling stop() for inactive watchdog");
          }
          isChecking = false;
      }
  
      /**
       * Execute the body of the Watchdog, triggering as appropriate.
       */
      public void run() {
  
          try {
              watchdogThread = Thread.currentThread();
  
              while ((!(Thread.currentThread().interrupted())) && (watchdogThread != null)) {
                  try {
                      if (!isChecking) {
                          if (getLogger().isDebugEnabled()) {
                              getLogger().debug("Watchdog " + Thread.currentThread().getName() + " is not active - going to exit.");
                          }
                          synchronized (this) {
                              if (!isChecking) {
                                  watchdogThread = null;
                              }
                              continue;
                          }
                      } else {
                          long currentTime = System.currentTimeMillis();
                          if (isReset) {
                              isReset = false;
                              lastReset = currentTime;
                          }
                          long timeToSleep = lastReset + timeout - currentTime;
                          if (watchdogThread != null) {
                              getLogger().debug("Watchdog " + watchdogThread.getName() + " has time to sleep " + timeToSleep);
                          } else {
                              getLogger().debug("Watchdog has time to sleep " + timeToSleep);
                          }
                          if (timeToSleep < 0) {
                              try {
                                  synchronized (this) {
                                      if ((isChecking) && (triggerTarget != null)) {
                                          triggerTarget.execute();
                                      }
                                      watchdogThread = null;
                                  }
                              } catch (Throwable t) {
                                  getLogger().error("Encountered error while executing Watchdog target.", t);
                              }
                              isChecking = false;
                              continue;
                          } else {
                              synchronized(this) {
                                  wait(timeToSleep);
                              }
                          }
                      }
                  } catch (InterruptedException ie) {
                  }
              }
  
              synchronized( this ) {
                  watchdogThread = null;
              }
          } finally {
              // Ensure that the thread is in a non-interrupted state when it gets returned
              // to the pool.
              Thread.currentThread().interrupted();
          }
          getLogger().debug("Watchdog " + Thread.currentThread().getName() + " is exiting run().");
      }
  
      /**
       * @see org.apache.avalon.framework.activity.Disposable#dispose()
       */
      public void dispose() {
          synchronized(this) {
              isChecking = false;
              if (watchdogThread != null) {
                  getLogger().debug("Calling disposeWatchdog() " + watchdogThread.getName());
              } else {
                  getLogger().debug("Calling disposeWatchdog() for inactive watchdog");
              }
              if (watchdogThread != null) {
                  watchdogThread = null;
                  notifyAll();
              }
              if (triggerTarget instanceof Disposable) {
                  ((Disposable)triggerTarget).dispose();
              }
              triggerTarget = null;
          }
      }
  }
  
  
  
  1.1                  jakarta-james/src/java/org/apache/james/util/watchdog/SchedulerWatchdogFactory.java
  
  Index: SchedulerWatchdogFactory.java
  ===================================================================
  /*
   * Copyright (C) The Apache Software Foundation. All rights reserved.
   *
   * This software is published under the terms of the Apache Software License
   * version 1.1, a copy of which has been included  with this distribution in
   * the LICENSE file.
   */
  package org.apache.james.util.watchdog;
  
  import org.apache.avalon.cornerstone.services.scheduler.PeriodicTimeTrigger;
  import org.apache.avalon.cornerstone.services.scheduler.Target;
  import org.apache.avalon.cornerstone.services.scheduler.TimeScheduler;
  
  /**
   * This class is a factory to produce Watchdogs, each of which is associated
   * with a single TimeScheduler Target and a TimeScheduler object.
   *
   * @author Peter M. Goldstein <fa...@alum.mit.edu>
   */
  public class SchedulerWatchdogFactory implements WatchdogFactory {
  
      /**
       * The thread pool used to generate InaccurateTimeoutWatchdogs
       */
      private TimeScheduler myTimeScheduler;
  
      private long timeout = -1;
  
      /**
       * Sets the TimeScheduler used to generate Watchdogs.
       */
      public void setTimeScheduler(TimeScheduler theTimeScheduler) {
          myTimeScheduler = theTimeScheduler;
      }
  
      /**
       * @see org.apache.james.util.watchdog.WatchdogFactory#getWatchdog(WatchdogTarget)
       */
      public Watchdog getWatchdog(WatchdogTarget theTarget) {
          return new SchedulerWatchdog(theTarget);
      }
  
      /**
       * An inner class that acts as an adaptor between the Watchdog
       * interface and the TimeScheduler interface.
       */
      private class SchedulerWatchdog implements Watchdog {
  
          /**
           * The in-scheduler identifier for this trigger.
           */
          private String triggerID = null;
  
          /**
           * The WatchdogTarget that is passed in when this
           * SchedulerWatchdog is initialized
           */
          private WatchdogTarget theWatchdogTarget;
  
          /**
           * Constructor for the SchedulerWatchdog
           *
           * @param theTarget the target triggered by this Watchdog
           */
          SchedulerWatchdog(WatchdogTarget theTarget) {
              // TODO: This should be made more robust then just
              //       using toString()
              triggerID = this.toString();
              theWatchdogTarget = theTarget;
          }
  
          /**
           * Start this Watchdog, causing it to begin monitoring.  The Watchdog can
           * be stopped and restarted.
           */
          public void start() {
              PeriodicTimeTrigger theTrigger = new PeriodicTimeTrigger((int)SchedulerWatchdogFactory.this.timeout, -1);
              Target theTarget = new Target() {
                                      public void targetTriggered(String targetID) {
                                          theWatchdogTarget.execute();
                                      }
                                 };
              SchedulerWatchdogFactory.this.myTimeScheduler.addTrigger(triggerID, theTrigger, theTarget);
          }
  
          /**
           * Reset this Watchdog.  Resets any conditions in the implementations
           * (time to expiration, etc.) to their original values
           */
          public void reset() {
              SchedulerWatchdogFactory.this.myTimeScheduler.resetTrigger(triggerID);
          }
  
          /**
           * Stop this Watchdog, terminating the monitoring condition.  The monitor
           * can be restarted with a call to startWatchdog.
           */
          public void stop() {
              SchedulerWatchdogFactory.this.myTimeScheduler.removeTrigger(triggerID);
          }
      }
  
  }
  
  
  
  1.1                  jakarta-james/src/java/org/apache/james/util/watchdog/ThreadPerWatchdogFactory.java
  
  Index: ThreadPerWatchdogFactory.java
  ===================================================================
  /*
   * Copyright (C) The Apache Software Foundation. All rights reserved.
   *
   * This software is published under the terms of the Apache Software License
   * version 1.1, a copy of which has been included  with this distribution in
   * the LICENSE file.
   */
  package org.apache.james.util.watchdog;
  
  import org.apache.avalon.excalibur.pool.DefaultPool;
  import org.apache.avalon.excalibur.pool.HardResourceLimitingPool;
  import org.apache.avalon.excalibur.pool.ObjectFactory;
  import org.apache.avalon.excalibur.pool.Pool;
  import org.apache.avalon.excalibur.pool.Poolable;
  import org.apache.avalon.excalibur.thread.ThreadPool;
  import org.apache.avalon.framework.activity.Disposable;
  import org.apache.avalon.framework.activity.Initializable;
  import org.apache.avalon.framework.logger.AbstractLogEnabled;
  import org.apache.avalon.framework.logger.LogEnabled;
  
  /**
   * This class is a factory to produce Watchdogs, each of which is associated
   * with a single thread.
   *
   * @author Peter M. Goldstein <fa...@alum.mit.edu>
   */
  public class ThreadPerWatchdogFactory
      extends AbstractLogEnabled
      implements WatchdogFactory {
  
      /**
       * The thread pool used to generate InaccurateTimeoutWatchdogs
       */
      private ThreadPool myThreadPool;
  
      /**
       * The watchdog timeout for Watchdogs generated by this factory
       */
      private long timeout;
  
      /**
       * Creates the factory and sets the thread pool used to generate
       * InaccurateTimeoutWatchdogs.
       */
      public ThreadPerWatchdogFactory(ThreadPool theThreadPool, long timeout) {
          myThreadPool = theThreadPool;
          if (theThreadPool == null) {
              throw new IllegalArgumentException("The thread pool for the ThreadPerWatchdogFactory cannot be null.");
          }
          myThreadPool = theThreadPool;
          this.timeout = timeout;
      }
  
      /**
       * @see org.apache.james.util.watchdog.WatchdogFactory#getWatchdog(WatchdogTarget)
       */
      public Watchdog getWatchdog(WatchdogTarget theTarget)
              throws Exception {
          InaccurateTimeoutWatchdog watchdog = new InaccurateTimeoutWatchdog(timeout, theTarget, myThreadPool);
          watchdog.enableLogging(getLogger());
          return watchdog;
      }
  }
  
  
  
  1.1                  jakarta-james/src/java/org/apache/james/util/watchdog/Watchdog.java
  
  Index: Watchdog.java
  ===================================================================
  /*
   * Copyright (C) The Apache Software Foundation. All rights reserved.
   *
   * This software is published under the terms of the Apache Software License
   * version 1.1, a copy of which has been included  with this distribution in
   * the LICENSE file.
   */
  package org.apache.james.util.watchdog;
  
  /**
   * This interface represents an abstract watchdog process that serves to
   * monitor a situation and triggers an action under an implementation-specific
   * trigger condition.
   *
   * @author Andrei Ivanov
   * @author Peter M. Goldstein <fa...@alum.mit.edu>
   */
  public interface Watchdog {
  
      /**
       * Start this Watchdog, causing it to begin monitoring.  The Watchdog can
       * be stopped and restarted.
       */
      void start();
  
      /**
       * Reset this Watchdog.  Resets any conditions in the implementations
       * (time to expiration, etc.) to their original values
       */
      void reset();
  
      /**
       * Stop this Watchdog, terminating the monitoring condition.  The monitor
       * can be restarted with a call to startWatchdog.
       */
      void stop();
  }
  
  
  
  1.1                  jakarta-james/src/java/org/apache/james/util/watchdog/WatchdogFactory.java
  
  Index: WatchdogFactory.java
  ===================================================================
  /*
   * Copyright (C) The Apache Software Foundation. All rights reserved.
   *
   * This software is published under the terms of the Apache Software License
   * version 1.1, a copy of which has been included  with this distribution in
   * the LICENSE file.
   */
  package org.apache.james.util.watchdog;
  
  /**
   * This interface represents a factory for producing Watchdogs.
   *
   * @author Andrei Ivanov
   * @author Peter M. Goldstein <fa...@alum.mit.edu>
   */
  public interface WatchdogFactory {
  
      /**
       * Gets a Watchdog
       *
       * @param theTarget the WatchdogTarget to be triggered upon expiration
       */
      Watchdog getWatchdog(WatchdogTarget theTarget) throws Exception;
  
  }
  
  
  
  1.1                  jakarta-james/src/java/org/apache/james/util/watchdog/WatchdogTarget.java
  
  Index: WatchdogTarget.java
  ===================================================================
  /*
   * Copyright (C) The Apache Software Foundation. All rights reserved.
   *
   * This software is published under the terms of the Apache Software License
   * version 1.1, a copy of which has been included  with this distribution in
   * the LICENSE file.
   */
  package org.apache.james.util.watchdog;
  
  import org.apache.avalon.cornerstone.services.scheduler.Target;
  
  /**
   * This interface represents an action to be triggered by a watchdog process.
   *
   * @author Andrei Ivanov
   * @author Peter M. Goldstein <fa...@alum.mit.edu>
   */
  public interface WatchdogTarget {
  
      void execute();
  }
  
  
  
  1.1                  jakarta-james/src/java/org/apache/james/util/watchdog/package.html
  
  Index: package.html
  ===================================================================
  <body>
  <p>Provides classes that define and implement a resettable Watchdog interface as well as related classes.  It might be desirable to have these classes eventually moved to Cornerstone.</p>
  </body>
  
  
  

--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: latest checkin..

Posted by Harmeet Bedi <ha...@kodemuse.com>.
----- Original Message -----
From: "Noel J. Bergman" <no...@devtech.com>
> Harmeet,
>
> I have reviewed your proposal.  Here is my quick take on it, as a proposed
> code change:
>
>   - AuthState         +1
>
> This just takes existing data from the handler, and encapsulates it as a
> Java Bean.
>
>   - AuthService       +1
>
> This simply takes two methods from the handler, and separates them as an
> interface.
>
>   - AuthServiceImpl   conditional
>
> You have eliminated the stateful problems, but it remains to be seen what
> you do with this class.  As I understand it, an instance of AuthService
> would be passed to the NNTPHandler through its service specific
> configuration object.
>
> Apparently, Peter objects to exposing AuthService to other James
components,
> so if this is a Block in james-assembly.xml, he has stated that he intends
> to veto it.
> I'm curious to see what other folks have to say.  However, if
> you hide the AuthService inside NNTPServer, e.g.,
>
>     NNTPServer()       { ... authService = new AuthServiceImpl(); ... }
>     compose(...)       { ... authService.compose(...); ...            }
>     configure(...)     { ... authService.configure(...); ...          }
>     enableLogging(...) { super(logger); setupLogger(authService); ... }
>
> or equivalent means, then that might address the objections that I have
read
> so far.  All of this presumes, of course, that the code works, otherwise
it
> would need to be fixed.
>


I was thinking along similar lines after the last offline conversion,
however wasn't sure if I should do it right away or defer when the email
flow started.

I was also thinking that configuration can be centralized, but the actual
code
can exist as a service. This would give ease of configuration and
pluggability.




> However, before you invest much more into this effort, consider the
> following.
>

>
> As I understand it the primary purpose for the AuthService is that you
want
> to decouple handlers from repositories: (a) basically don't trust the
> repository interface, and (b) you want to allow an LDAP or Radius
> authentication server to be used.  If someone deploying James has that
need,
> it can be addressed.  In the meantime, you can easily isolate the
> AuthService inside the NNTPServer, allowing NNTPHandler to remain entirely
> unchanged, which is your primary goal.
>
> One reason for limiting the investment of time and energy into AuthService
> is that if JAAS ends up being used in v3, then it is likely that
AuthService
> would disappear again forever, since JAAS has its own means by which
service
> providers are installed.
>


JAAS is possible and a good idea. But JAAS may not be overkill for some.

A simple authentication service that allows folks to plugin authentication
without understanding Avalon User Respositories may be valuable. Implement 2
methods to plugin authentication seems simple to explain to anyone who may
need to do this.

AuthService is also minor change on the current code. It has been there but
stateful and broken so in essence it is a fix more than anything else.
Regd. effort - has already been done. I was trying to write automated
regression tests
to verify things better for now and future.



I prefer that we defer this change and any scheduler changes
till 2.1 is complete. I would like to minimize chances of the kind of
interaction that has gone on recently.

> Your thoughts, Kind Sir?

Hopefully someday I will be as kind and graceful as you. :-)


Harmeet


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: latest checkin..

Posted by "Noel J. Bergman" <no...@devtech.com>.
Harmeet,

I have reviewed your proposal.  Here is my quick take on it, as a proposed
code change:

  - AuthState         +1

This just takes existing data from the handler, and encapsulates it as a
Java Bean.

  - AuthService       +1

This simply takes two methods from the handler, and separates them as an
interface.

  - AuthServiceImpl   conditional

You have eliminated the stateful problems, but it remains to be seen what
you do with this class.  As I understand it, an instance of AuthService
would be passed to the NNTPHandler through its service specific
configuration object.

Apparently, Peter objects to exposing AuthService to other James components,
so if this is a Block in james-assembly.xml, he has stated that he intends
to veto it.  I'm curious to see what other folks have to say.  However, if
you hide the AuthService inside NNTPServer, e.g.,

    NNTPServer()       { ... authService = new AuthServiceImpl(); ... }
    compose(...)       { ... authService.compose(...); ...            }
    configure(...)     { ... authService.configure(...); ...          }
    enableLogging(...) { super(logger); setupLogger(authService); ... }

or equivalent means, then that might address the objections that I have read
so far.  All of this presumes, of course, that the code works, otherwise it
would need to be fixed.

However, before you invest much more into this effort, consider the
following.

As I understand it the primary purpose for the AuthService is that you want
to decouple handlers from repositories: (a) basically don't trust the
repository interface, and (b) you want to allow an LDAP or Radius
authentication server to be used.  If someone deploying James has that need,
it can be addressed.  In the meantime, you can easily isolate the
AuthService inside the NNTPServer, allowing NNTPHandler to remain entirely
unchanged, which is your primary goal.

One reason for limiting the investment of time and energy into AuthService
is that if JAAS ends up being used in v3, then it is likely that AuthService
would disappear again forever, since JAAS has its own means by which service
providers are installed.

Your thoughts, Kind Sir?

	--- Noel


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: latest checkin..

Posted by Harmeet Bedi <ha...@kodemuse.com>.
----- Original Message -----
From: "Noel J. Bergman" <no...@devtech.com>
> > > > Sceduler implementation has already been verified. You made a number
> > > > of fixes too in watchdog.
> > > Noel seems to disagree with this statement.  He states that there is
at
> > > least one open bug in your scheduler implementation.
> > The only thing I know of is optimization issues not correctness ones.
>
> Harmeet, I have mentioned at least twice this week that there is one
> outstanding issue that prevents your Scheduler from being a drop-in for
the
> Avalon Scheduler.  I'll remind you.
>
> The problem is/was that it doesn't handle inserting a time earlier than
the
> head of the list.  And since there are other components that might use a
> Scheduler AS a scheduler, e.g., FetchPOP, that bug needs to be fixed.
>


Sure, will send it around tonight or tomorrow.
Got lost in the vast flood of email, code and life.
thanks for the reminder.

> Between Peter and myself, we have already made the changes to permit a
> Scheduler to be used for Watchdog implementation, but you need to finish
> your side of the equation.

cool,

Harmeet


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: latest checkin..

Posted by "Noel J. Bergman" <no...@devtech.com>.
> > > Sceduler implementation has already been verified. You made a number
> > > of fixes too in watchdog.
> > Noel seems to disagree with this statement.  He states that there is at
> > least one open bug in your scheduler implementation.
> The only thing I know of is optimization issues not correctness ones.

Harmeet, I have mentioned at least twice this week that there is one
outstanding issue that prevents your Scheduler from being a drop-in for the
Avalon Scheduler.  I'll remind you.

The problem is/was that it doesn't handle inserting a time earlier than the
head of the list.  And since there are other components that might use a
Scheduler AS a scheduler, e.g., FetchPOP, that bug needs to be fixed.

Between Peter and myself, we have already made the changes to permit a
Scheduler to be used for Watchdog implementation, but you need to finish
your side of the equation.

	--- Noel


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: latest checkin..

Posted by Harmeet Bedi <ha...@kodemuse.com>.
----- Original Message -----
From: "Peter M. Goldstein" <pe...@yahoo.com>
>
> Harmeet,
>
> > > It was resolved, as it always is, in a matter of hours
> > > of being reported.  Not days.  Or months.
> >
> > Agreed, but you broke it, I reported it and you fixed it. I could have
>
> > fixed it too.
>
> Just like you could've fixed the broken AuthService anytime in the last
> nine months.  But you didn't.  And thus it doesn't count.  You see,
> saying you're going to do something doesn't cut any mustard when you've
> got no follow through.
>
> This is in fact just like AuthService.  You claimed to be able to write
> a service that would replace it.  You never submitted it for
> consideration.  It doesn't exist.  You never bothered to make it exist.
> It's much easier to not do the work and play the victim.


Alright Peter here is the attached service. I basically decided to let you
do your thing and that was not due to laziness, but just did not want the
conflict.


>
> Test code for test code's sake is useless.  It's only when it's part of
> a comprehensive notion of testing - and when those test cases are
> clearly labeled - that the testing is at all worthwhile.  See any of
> Noel's emails regarding test code.

It would be good to have some contributions from you in this area since you
have such strong views on this.
As far as I know the testing ideas are in line what has been talked off and
I have been commuicating with everyone interested in this topic to make it
better.


>
>
> > > ii) A superfluous class (CRLFWriter.java) that didn't even address a
>
> > > documented issue in the NNTP code.  This issue means that the
> > > CRLFWriter code was wrong.
> >
> > This is no longer there. As soon as I saw something eqivalent I
> > removed it. This was yesterday or a couple of days back.
>
> Exactly.  So it wasn't a useful contribution.  Of course you didn't
> bother to actually read the code base to investigate this...


No it wasn't. I had read the code before but did not remember. What is your
point ?. I added a class, realized it was redundant and then removed it. So
is that an offence in Apache ?


>
>
> > >
> > > iii) An incorrect scheduler that exhibited almost exactly the same
> > > behavior as the current Scheduler
> > >
> > > iv) Another Scheduler implementation that no one but you seems to
> > > think we need, and that you haven't put the time in to build and
> > > test with Noel.
> >
> > Sceduler implementation has already been verified. You made a number
> > of fixes too in watchdog.
>
> Noel seems to disagree with this statement.  He states that there is at
> least one open bug in your scheduler implementation.



The only thing I know of is optimization issues not correctness ones. Did
not want to optimize as I was not sure it would be production code.



> Moreover, as far
> as I can tell from the posts, you've never sent him a .sar for testing.


I usu. zip the the James dist and put it up. Sent Noel and cc'd list a few
times.


> So we have no idea, save for your word (which you also gave for the
> Timer implementation of the scheduler, as I recall), that the thing
> works properly under load.  It may.  It may not.


Noel's test confirmed that it stays up under load.


> > > A regression indeed.  And fixed.  In an hour.  Not nine months.
> >
> > After I posted the exact snippet that exhibitted the problem. Would
> > have been nicer if you would have once tested it yourself or
> > discovered the snippet rather than adding code and breaking the
> > server.
>
>
> And it would've been nice if you hadn't implemented an architecturally
> unsound AuthService.  We all make mistakes.


true.


> The difference is that I
> fix my own.  You do not.


You are fixing things that I wrote 9 months back and after I have sent the
exact problem and place where it is broken.


>
> So to sum up, in the entirety of 2002 you haven't contributed a single
> useful line of code or documentation.  Yet you wish to block those
> who've been working diligently on the project for months.  It all comes
> back to ethics...
>
> >
> > As the NNTP code stood today morning at least, it was much worse than
> > anytime in last 9 months. Auth was broken for 9 months but in the last
>
> > few days everything was broken. I am trying to test and fix.
>
> In a word, this is a lie.
>
> There are zero (that's none) outstanding bugs against the current code
> base.  All the issues you reported have been addressed - quickly and
> efficiently.  What's your issue?
>
> Name your issue and I'll look at it.  But the server works - and was
> tested a great deal more than it ever was before.


It sometimes takes a lot longer to find the issues and narrowed to 5 lines
of code than fix the issue.

>
> Just the sheer unmitigated gall of this response is amazing.  We have
> here a committer who abandoned the product for nine months, with a slew
> of open critical bugs against his nominally owned component.

This happens. I did make significant contributions then got busy in other
things. You cannot have a person contribute in open source and fault him for
not contributing at the same pace or being around.


Rest of the email doesn't merit a response, but I do wish you could cool
down.


Harmeet

RE: latest checkin..

Posted by "Peter M. Goldstein" <pe...@yahoo.com>.
Harmeet,

> > It was resolved, as it always is, in a matter of hours
> > of being reported.  Not days.  Or months.
> 
> Agreed, but you broke it, I reported it and you fixed it. I could have

> fixed it too.

Just like you could've fixed the broken AuthService anytime in the last
nine months.  But you didn't.  And thus it doesn't count.  You see,
saying you're going to do something doesn't cut any mustard when you've
got no follow through.

This is in fact just like AuthService.  You claimed to be able to write
a service that would replace it.  You never submitted it for
consideration.  It doesn't exist.  You never bothered to make it exist.
It's much easier to not do the work and play the victim.

You simply ignored all the posed technical points and rather than trying
to address them, you went off and sulked.  And then you took out your
passive-aggressive tendencies on Noel's committer vote.  Lovely.

> 
> > Perhaps this is the core of the issue.  You seem to think that 
> > you're work contributed prior to this release allows you dictatorial

> > control over the code.
> 
> I have reported the issue and exact code where things were broken. If 
> I felt what you are saying, I woundn't have been sending code 
> problematic snippets
> to the list.
> 
> 
> > i) Test code with an unmarked protocol script in violation of the 
> > RFC
> 
> Some test code is better than none.
> If there is a test script that voilates RFC why don't you suggest a 
> patch or add tests.

Test code for test code's sake is useless.  It's only when it's part of
a comprehensive notion of testing - and when those test cases are
clearly labeled - that the testing is at all worthwhile.  See any of
Noel's emails regarding test code.


> > ii) A superfluous class (CRLFWriter.java) that didn't even address a

> > documented issue in the NNTP code.  This issue means that the 
> > CRLFWriter code was wrong.
> 
> This is no longer there. As soon as I saw something eqivalent I 
> removed it. This was yesterday or a couple of days back.

Exactly.  So it wasn't a useful contribution.  Of course you didn't
bother to actually read the code base to investigate this...

 
> >
> > iii) An incorrect scheduler that exhibited almost exactly the same 
> > behavior as the current Scheduler
> >
> > iv) Another Scheduler implementation that no one but you seems to 
> > think we need, and that you haven't put the time in to build and 
> > test with Noel.
> 
> Sceduler implementation has already been verified. You made a number 
> of fixes too in watchdog.

Noel seems to disagree with this statement.  He states that there is at
least one open bug in your scheduler implementation.  Moreover, as far
as I can tell from the posts, you've never sent him a .sar for testing.
So we have no idea, save for your word (which you also gave for the
Timer implementation of the scheduler, as I recall), that the thing
works properly under load.  It may.  It may not.  But you haven't put in
the effort to test and possibly fix.  As far as the Watchdog goes, I
have.  It works and has been demonstrated to work by other people.

> > A regression indeed.  And fixed.  In an hour.  Not nine months.
> 
> After I posted the exact snippet that exhibitted the problem. Would 
> have been nicer if you would have once tested it yourself or 
> discovered the snippet rather than adding code and breaking the 
> server.


And it would've been nice if you hadn't implemented an architecturally
unsound AuthService.  We all make mistakes.  The difference is that I
fix my own.  You do not.

So to sum up, in the entirety of 2002 you haven't contributed a single
useful line of code or documentation.  Yet you wish to block those
who've been working diligently on the project for months.  It all comes
back to ethics...
 
> 
> As the NNTP code stood today morning at least, it was much worse than 
> anytime in last 9 months. Auth was broken for 9 months but in the last

> few days everything was broken. I am trying to test and fix.

In a word, this is a lie.

There are zero (that's none) outstanding bugs against the current code
base.  All the issues you reported have been addressed - quickly and
efficiently.  What's your issue?  

Name your issue and I'll look at it.  But the server works - and was
tested a great deal more than it ever was before.

Just the sheer unmitigated gall of this response is amazing.  We have
here a committer who abandoned the product for nine months, with a slew
of open critical bugs against his nominally owned component. 

For god's sake, one of them was a typo in a string.  It took a grand
total of 20 seconds for me to find it.

And yet it's like every nugget of code he wrote came down from the
mountain with Moses.  The attitude just leaves me speechless.

And we once again come back to the same point - if you don't like the
changes in James Harmeet, use an older version.  You haven't contributed
one whit to this or to the last version (2.0a3), so I don't see what
you're complaining about.

--Peter



--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: latest checkin..

Posted by Harmeet Bedi <ha...@kodemuse.com>.
----- Original Message -----
From: "Peter M. Goldstein" <pe...@yahoo.com>
> A NPE that only occurred in a fresh server, not in a test server.
> Largely because the previous NNTP code mixed up the directory lookup and
> validation code.

NPE and server not starting was not a problem in earlier code.

> It was resolved, as it always is, in a matter of hours
> of being reported.  Not days.  Or months.

Agreed, but you broke it, I reported it and you fixed it. I could have fixed
it too.


> Perhaps this is the core of the issue.  You seem to think that you're
> work contributed prior to this release allows you dictatorial control
> over the code.

I have reported the issue and exact code where things were broken. If I felt
what you are saying, I woundn't have been sending code problematic snippets
to the list.


> i) Test code with an unmarked protocol script in violation of the RFC

Some test code is better than none.
If there is a test script that voilates RFC why don't you suggest a patch or
add tests.

>
> ii) A superfluous class (CRLFWriter.java) that didn't even address a
> documented issue in the NNTP code.  This issue means that the CRLFWriter
> code was wrong.

This is no longer there. As soon as I saw something eqivalent I removed it.
This was yesterday or a couple of days back.

>
> iii) An incorrect scheduler that exhibited almost exactly the same
> behavior as the current Scheduler
>
> iv) Another Scheduler implementation that no one but you seems to think
> we need, and that you haven't put the time in to build and test with
> Noel.

Sceduler implementation has already been verified. You made a number of
fixes too in watchdog.

> A regression indeed.  And fixed.  In an hour.  Not nine months.

After I posted the exact snippet that exhibitted the problem. Would have
been nicer if you would have once tested it yourself or discovered the
snippet rather than adding code and breaking the server.



As the NNTP code stood today morning at least, it was much worse than
anytime in last 9 months. Auth was broken for 9 months but in the last few
days everything was broken. I am trying to test and fix.

Harmeet


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: latest checkin..

Posted by "Peter M. Goldstein" <pe...@yahoo.com>.

Harmeet,

> > It was run a great deal more than once.  Of course in my config, the
var
> > is deliberately kept at root.  So I wouldn't see this issue.
> 
> The last refactoring had caused a NullPointerException and Sever not
> starting.

A NPE that only occurred in a fresh server, not in a test server.
Largely because the previous NNTP code mixed up the directory lookup and
validation code.  It was resolved, as it always is, in a matter of hours
of being reported.  Not days.  Or months.
 
> MODE READER is the first command sent by an NNTP Client.
> As you can probabaly see it is broken...
> 
> Your intent is good and changes knowledgeable, but neither justifies
> removing work done by others(effective veto over other folks work) nor
> does
> it justify breaking things. At least have a test or two. I could have
> checked in the auth fix a few hours back but I am holding off till I
have
> a
> regression test...

What work?

If you want to keep your current NNTP service, you're welcome to use an
older version James.  I contributed absolutely nothing prior to James
2.0a3, so you're welcome to use that.  It's wrong, but you haven't
contributed any changes to fix it, so I don't see how you have any
problem with it.

Perhaps this is the core of the issue.  You seem to think that you're
work contributed prior to this release allows you dictatorial control
over the code.

These are the hard facts.  James has almost no one using NNTP because it
simply didn't work.  Check the traffic on james-user.  The only mention
of NNTP in the last three months was a user rejecting the implementation
as unsuitable for production.  No one was maintaining it.  There was a
typo bug in the SSL connection that had gone unattended for nine months.
There was the AuthService bug, an architectural problem, that lay
dormant for the same nine months.  Your return coincided almost exactly
with actual repairs to the NNTP server to bring it up to a production
quality server (or close to it).

Since your return you have contributed:

i) Test code with an unmarked protocol script in violation of the RFC

ii) A superfluous class (CRLFWriter.java) that didn't even address a
documented issue in the NNTP code.  This issue means that the CRLFWriter
code was wrong.

iii) An incorrect scheduler that exhibited almost exactly the same
behavior as the current Scheduler

iv) Another Scheduler implementation that no one but you seems to think
we need, and that you haven't put the time in to build and test with
Noel.

So whose work am I wasting?  Certainly not yours.
 
> Here is the MODE READER implementation. Argument as sent by outlook
> express
> is null.
> yielding
> S: 501 Syntax error - unexpected parameter
> S: 200 Posting Permitted
> 
>     private void doMODEREADER(String argument) {
>         // 7.2
>         if ( argument != null ) {
>             writer.println("501 Syntax error - unexpected parameter");
>         }
>         writer.println(theConfigData.getNNTPRepository().isReadOnly()
>                        ? "201 Posting Not Permitted" : "200 Posting
> Permitted");
>     }

A regression indeed.  And fixed.  In an hour.  Not nine months.

--Peter



--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: latest checkin..

Posted by Harmeet Bedi <ha...@kodemuse.com>.
----- Original Message -----
From: "Peter M. Goldstein" <pe...@yahoo.com>
> It was run a great deal more than once.  Of course in my config, the var
> is deliberately kept at root.  So I wouldn't see this issue.

The last refactoring had caused a NullPointerException and Sever not
starting.

MODE READER is the first command sent by an NNTP Client.
As you can probabaly see it is broken...

Your intent is good and changes knowledgeable, but neither justifies
removing work done by others(effective veto over other folks work) nor does
it justify breaking things. At least have a test or two. I could have
checked in the auth fix a few hours back but I am holding off till I have a
regression test...


Here is the MODE READER implementation. Argument as sent by outlook express
is null.
yielding
S: 501 Syntax error - unexpected parameter
S: 200 Posting Permitted

    private void doMODEREADER(String argument) {
        // 7.2
        if ( argument != null ) {
            writer.println("501 Syntax error - unexpected parameter");
        }
        writer.println(theConfigData.getNNTPRepository().isReadOnly()
                       ? "201 Posting Not Permitted" : "200 Posting
Permitted");
    }

Harmeet


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: latest checkin..

Posted by "Peter M. Goldstein" <pe...@yahoo.com>.
Harmeet,

Auth issue isn't a regression, as discussed in my previous email.

> - NNTP Stores should be under apps, now they are created under root
> directory.

This is correct, but the identification of cause is off.

The previous code was broken - it didn't handle absolute URLs correctly.
So it places file:/// URLs in the apps directory, contrary to every
other URL in the system.  

Correcting the default configuration so that the URLs are relative by
default.

> Possible cleanup regressions... Nothing to do with scheduler or
service
> patch.
 
> Fixing second issue, first one is a bit more problematic and would
have
> been
> caught if the server was started once post cleanup/redesign. Will look
> into
> it and fix soon.

Second issue is fixed, thank you.

It was run a great deal more than once.  Of course in my config, the var
is deliberately kept at root.  So I wouldn't see this issue.

--Peter



--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: latest checkin..

Posted by Harmeet Bedi <ha...@kodemuse.com>.
2 more problems.

- NNTP Stores should be under apps, now they are created under root
directory.
- The hook to see input output commands is gone. There does not seem to be
any way to dump output from server to log file.

Possible cleanup regressions... Nothing to do with scheduler or service
patch.

Fixing second issue, first one is a bit more problematic and would have been
caught if the server was started once post cleanup/redesign. Will look into
it and fix soon.

Harmeet


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: latest checkin..

Posted by Harmeet Bedi <ha...@kodemuse.com>.
Instead of code like this

        theWatchdogFactory = new ThreadPerWatchdogFactory(threadPool,
timeout);
Would it be better to have 'WatchdogFactory' as a Avalon Block ?

To replace the impl would require code changes in the *Server files.
Having a pluggable block would be better and more in line with the component
oriented philosophy in the rest of James code.

Harmeet


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: latest checkin..

Posted by "Peter M. Goldstein" <pe...@yahoo.com>.
Harmeet,

> One flaw is that if the authentication credentials are passed in again
the
> alreadyAuthenticated state needs to be cleared.
> 
> 

Actually this description of the problem is not necessarily correct.
>From section 3.1.1 of RFC 2980, which discusses the AUTHINFO command,
specifies a set of as of yet unimplemented behavior (rejecting out of
order authentication state, possibly rejecting reauthentication).  I'm
putting in the requisite return codes.  Missed them because I was
focusing on the original NNTP RFC.

--Peter
 



--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: latest checkin..

Posted by "Noel J. Bergman" <no...@devtech.com>.
> It's a basically three line change to change the current code to allow
> re-authentication (rather than returning a 482, the user, pass, and
> isAlreadyAuthenticated values get wiped).  I don't really care - so long
> as the other behavior (handling double USER submission, out of order,
> double PASSWORD) is there.

>From the look of Harmeet's initial posting, it is a single object that would
be associated with the handler, so per session.  I'm not sure that we really
need another object, but what's one more pair of strings?

	--- Noel


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: latest checkin..

Posted by "Peter M. Goldstein" <pe...@yahoo.com>.
Noel,

> Is there any additional language that refers to a prefered solution,
or to
> any additional semantics for re-authentication?  Without the RFC
> specifying
> a preference, or common practice if the RFC is neutral, I don't
> particularly
> care which optional behavior is manifested in the code, so long as it
> works.
> Do anyone else?  Harmeet seems to have a preference.

Nothing documented in the RFC.  There might be something somewhere else.

It's a basically three line change to change the current code to allow
re-authentication (rather than returning a 482, the user, pass, and
isAlreadyAuthenticated values get wiped).  I don't really care - so long
as the other behavior (handling double USER submission, out of order,
double PASSWORD) is there.
 
> > This is a MAY, not a MUST, so it is indeed optional.
> 
> I generally find that MAY is used to specify minimally required
behavior,
> when alternatives are more complex and not universally necessary.

Yep.
 
--Peter



--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: latest checkin..

Posted by "Noel J. Bergman" <no...@devtech.com>.
> The only statement on reauthentication in the RFC is the following:

So the RFC language permits either an a priori 482 return or a
re-authentication.

> I chose to implement the specified behavior,
> because that's what is discussed in the RFC.

Is there any additional language that refers to a prefered solution, or to
any additional semantics for re-authentication?  Without the RFC specifying
a preference, or common practice if the RFC is neutral, I don't particularly
care which optional behavior is manifested in the code, so long as it works.
Do anyone else?  Harmeet seems to have a preference.

> This is a MAY, not a MUST, so it is indeed optional.

I generally find that MAY is used to specify minimally required behavior,
when alternatives are more complex and not universally necessary.

> Note that this is not a regression.

We know.

	--- Noel


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: latest checkin..

Posted by "Peter M. Goldstein" <pe...@yahoo.com>.
Noel et al,

> As far as I'm concerned, no one --- not me, not you, not Peter, no one
---
> should be submitting changes to the wire-level-protocol syntax and
> semantics
> without documenting the relevant RFC issues.  If there is disagreement
on
> how to interpret the RFC, we can check with other sources.  But this
is
> not
> a subjective matter.  There may be options that render multiple
choices
> correct, but that also needs to be demonstrated from the RFC.

The only statement on reauthentication in the RFC is the following:

"If a client attempts to reauthenticate, the server may return 482
response 
indicating that the new authentication data is rejected by the server."

This is a MAY, not a MUST, so it is indeed optional.  It would certainly
be possible to make this configurable.  I chose to implement the
specified behavior, because that's what is discussed in the RFC.

Note that this is not a regression.  Since the AuthService never worked
- try having two different authenticated users on James 2.0a3 and see
what happens when you do a re-authentication.  Bad, bad things happen.

--Peter



--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: latest checkin..

Posted by "Noel J. Bergman" <no...@devtech.com>.
> Would prefer to have deferred this discussion...
> Here it is:

You were the one who raise the discussion.  I didn't ask to see code
(finished or unfinished).  I asked to see a state description of the
behavior.  I can read the previous code to see what IT manifests the
behavior to be, I can read your new code to see what IT manifests the
behavior to be, but NEITHER is an RFC driven description of authentication
states and transitions.

This isn't a style issue, a refactoring issue, or some other subjective.
There is an RFC that dictates the correct behavior.  You said that the
current code does not model the correct behavior.  I'm not debating the
point with you.  I'm asking you for an RFC driven description of the proper
behavior.  If necessary, I'll read the RFC, myself.

As far as I'm concerned, no one --- not me, not you, not Peter, no one ---
should be submitting changes to the wire-level-protocol syntax and semantics
without documenting the relevant RFC issues.  If there is disagreement on
how to interpret the RFC, we can check with other sources.  But this is not
a subjective matter.  There may be options that render multiple choices
correct, but that also needs to be demonstrated from the RFC.

	--- Noel


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: latest checkin..

Posted by Harmeet Bedi <ha...@kodemuse.com>.
----- Original Message -----
From: "Noel J. Bergman" <no...@devtech.com>
regd. AuthState.
> AuthI'm looking at it now ... I don't see where it does what you say it
does.  I
> see two setters and the isAuthenticated() method:

Noel I have not been ready with this yet, but if are keen I'll post. Would
prefer to have deferred this discussion...

Here it is:

/**
 * Provides Authentication Credentials.
 *
 * Helps manage the Protocol Authentication state, resets
 * authenticated flag as need be.
 *
 * Helps avoid unnecessary re authentication.
 *
 */
public class AuthState
{
    private String userID;
    private String password;
    private boolean authenticated;

    // ---- methods invoked by authentication service -----
    /**
     * @return value of authenticated flag.
     */
    public boolean isAuthenticated() {
        return authenticated;
    }

    /**
     * Get the value of userID.
     * @return value of userID.
     */
    public String getUserID() {
        return userID;
    }

    /**
     * Get the value of password.
     * @return value of password.
     */
    public String getPassword() {
        return password;
    }

    /**
     * @param v  Value to assign to indicate authentication done or not.
     */
    void setAuthenticated(boolean  v) {
        this.authenticated = v;
    }

    /**
     * Set the value of userID. reset authentication state
     * @param v  Value to assign to userID.
     */
    void setUserID(String  v) {
        reset();
        this.userID = v;
    }

    /**
     * Set the value of password. reset authentication flag
     * @param v  Value to assign to password.
     */
    void setPassword(String  v) {
        this.authenticated = false;
        this.password = v;
    }

    /** reset to initial state */
    void reset() {
        userID = password = null;
        authenticated = false;
    }
}


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: latest checkin..

Posted by "Noel J. Bergman" <no...@devtech.com>.
> Had the AuthState object

I'm looking at it now ... I don't see where it does what you say it does.  I
see two setters and the isAuthenticated() method:

   public boolean isAuthenticated() {
       if ( requiredAuth ) {
           if  (userSet && passwordSet ) {
               return repo.test(user,password);
           } else {
               return false;
           }
       } else {
           return true;
       }
   }

Once the userSet or passwordSet are set it always checks.  They are never
cleared.  Therefore if requiredAuth is ever set, and once the user and
password are set, it always checks the repository.  Which is precisely the
complaint you initially raised this morning.

It appears that there is some question about WHAT the behavior should be,
not how to do it.  So, PLEASE submit the state machine you believe describes
the behavior.  We ought to resolve this issue at that level first, then make
any necessary code changes.

	--- Noel


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: latest checkin..

Posted by Harmeet Bedi <ha...@kodemuse.com>.
----- Original Message -----
From: "Noel J. Bergman" <no...@devtech.com>
> Why don't you start by posting what you think the authentication state
> machine looks like?

I am almost there, need to do a bit more testing...

Had the AuthState object before AuthService, and then I started refactoring
James to have multiple service(from one single James specific service) and
probably got too excited and introduced AuthService bug.

http://cvs.apache.org/viewcvs.cgi/jakarta-james/src/java/org/apache/james/nn
tpserver/Attic/AuthState.java

I won't be exactly like this, but something similar. Should be in better
position to send out later today.

Harmeet


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: latest checkin..

Posted by "Noel J. Bergman" <no...@devtech.com>.
> > > Authentication is done against repository for
> > > Each NNTP Comamnd in a User Session.
> > > It should only be done once per user session.

> > Are you saying that the isAuthenticated() method
> > should short-circuit if it has already authenticated?

> if the authentication credentials are passed in again
> the alreadyAuthenticated state needs to be cleared.

Why don't you start by posting what you think the authentication state
machine looks like?

	--- Noel


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: latest checkin..

Posted by Harmeet Bedi <ha...@kodemuse.com>.
----- Original Message -----
From: "Noel J. Bergman" <no...@devtech.com>
> > Authentication is done against repository for
> > Each NNTP Comamnd in a User Session.
> > It should only be done once per user session.
>
> Are you saying that the isAuthenticated() method should short-circuit if
it
> has already authenticated?  Something like:
>
>  private boolean isAuthenticated() {
>    if (alreadyAuthenticated) {
>        return true;
>    } else {
>        ...
>    }
>

One flaw is that if the authentication credentials are passed in again the
alreadyAuthenticated state needs to be cleared.

I would prefer to separate protocol authentication state management from the
handler. Likely to fix this sometime today.

Harmeet


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: latest checkin..

Posted by "Noel J. Bergman" <no...@devtech.com>.
> Authentication is done against repository for
> Each NNTP Comamnd in a User Session.
> It should only be done once per user session.

Are you saying that the isAuthenticated() method should short-circuit if it
has already authenticated?  Something like:

 private boolean isAuthenticated() {
   if (alreadyAuthenticated) {
       return true;
   } else {
       ...
   }

	--- Noel


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: latest checkin..

Posted by Harmeet Bedi <ha...@kodemuse.com>.
There seems to be at least one problem recent changes:
  Authentication is done against repository for Each NNTP Comamnd in a User
Session.

It should only be done once per user session.

Harmeet


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: latest checkin..

Posted by "Peter M. Goldstein" <pe...@yahoo.com>.
Harmeet,

> Don't know what to say to that... Peter is Peter.
> There is no reason to burn oneself over volunteer work.
> We are all trying to build the right thing, and it is better to work
> together than get upset.

And this is exactly why you shouldn't be a committer.

As I've said before, this is not a game.  Those of us who devote our
time and energy to making James an enterprise class piece of software do
not consider it a game.  If you do (as you clearly do), get out.

I take my duties seriously - be they volunteer or otherwise.  My
professionalism is a fundamental part of me, and it doesn't matter
whether I'm getting paid or not.  I take my responsibilities seriously
when I'm volunteering for charity, playing on a team, or working at a
job.  I simply don't understand those who don't...

How you can claim to be working together when you lie and commit slander
I will never know...

--Peter



--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: *** PLEASE STOP ***

Posted by Stephen McConnell <mc...@apache.org>.

Noel J. Bergman wrote:

>>Sorry about that. I was a bit ticked off and that may have showed.
>>I do intend to be positive, but it is hard sometimes not to push
>>back.
>>    
>>
>
>Well until everyone STOPS pushing back at each other, calms down, and
>focuses on code, this is going to continue to spiral, until it breaks.  And
>that is the last thing anyone wants.
>
>  
>
>>>Help is always appreciated, but at this point
>>>you're driving Peter bonkers,
>>>      
>>>
>
>Both of those are are pretty fair assessments.  But everyone probably owes
>everyone an apology for SOMETHING around here lately.  So I'll say it for
>everyone, so that no one has to step up and admit that he or she might have
>stepped over a line somewhere: I'M SORRY.  WE'RE ALL SORRY.
>
>Now ... let's not have any more personal attacks.  No more calling someone
>else a liar.  No more revisiting the past.  We've got code in the CVS now,
>and we want to move forward FROM HERE.
>
>Computers run code.  They don't care about egos, personality, or even
>perceptions of intelligent life.  They just run the code.  It either works,
>or it doesn't.
>
>As I understand it, the rules are that if someone, anyone, delivers good
>code then we are supposed to have a good reason for rejecting it.  On the
>other hand, the rule is that if code is submitted by someone, anyone, that
>we don't believe we can in good conscience support, then we are justified to
>give our technical reaons and reject it.
>

+1

But I'm really really impressed with the James level progress over here.

Cheers, Steve.

-- 

Stephen J. McConnell

OSM SARL
digital products for a global economy
mailto:mcconnell@osm.net
http://www.osm.net




--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: *** PLEASE STOP ***

Posted by "Noel J. Bergman" <no...@devtech.com>.
> It is surely part of the process that minor faults will
> be be corrected by commiters or contributed patches

Of course.  Bugs happen all the time.  Let he who has written no bugs ...
:-)

	--- Noel


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: *** PLEASE STOP ***

Posted by Danny Angus <da...@apache.org>.
> As I understand it, the rules are that if someone, anyone, delivers good
> code then we are supposed to have a good reason for rejecting it.

Seems reasonable to me, under the circumstances, if a change is shown to offer improvements over existing code, and is reasonably implemented, I think any objecton would have to be well grounded.
(It is surely part of the process that minor faults will be be corrected by commiters or contributed patches.)

d.


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


*** PLEASE STOP ***

Posted by "Noel J. Bergman" <no...@devtech.com>.
> Sorry about that. I was a bit ticked off and that may have showed.
> I do intend to be positive, but it is hard sometimes not to push
> back.

Well until everyone STOPS pushing back at each other, calms down, and
focuses on code, this is going to continue to spiral, until it breaks.  And
that is the last thing anyone wants.

> > Help is always appreciated, but at this point
> > you're driving Peter bonkers,

Both of those are are pretty fair assessments.  But everyone probably owes
everyone an apology for SOMETHING around here lately.  So I'll say it for
everyone, so that no one has to step up and admit that he or she might have
stepped over a line somewhere: I'M SORRY.  WE'RE ALL SORRY.

Now ... let's not have any more personal attacks.  No more calling someone
else a liar.  No more revisiting the past.  We've got code in the CVS now,
and we want to move forward FROM HERE.

Computers run code.  They don't care about egos, personality, or even
perceptions of intelligent life.  They just run the code.  It either works,
or it doesn't.

As I understand it, the rules are that if someone, anyone, delivers good
code then we are supposed to have a good reason for rejecting it.  On the
other hand, the rule is that if code is submitted by someone, anyone, that
we don't believe we can in good conscience support, then we are justified to
give our technical reaons and reject it.

	--- Noel


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: latest checkin..

Posted by Harmeet Bedi <ha...@kodemuse.com>.
----- Original Message -----
From: "Serge Knystautas" <se...@lokitech.com>
> I'm sorry, but your posts just aren't coming across as positively as I
think
> you intend them.

Sorry about that. I was a bit ticked off and that may have showed.
I do intend to be positive, but it is hard sometimes not to push back.

> Help is always appreciated, but at this point
> you're driving Peter bonkers,

Don't know what to say to that... Peter is Peter.
There is no reason to burn oneself over volunteer work.
We are all trying to build the right thing, and it is better to work
together than get upset.

> and I don't think Noel is too happy either.

I hope that is not true.

Harmeet


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: latest checkin..

Posted by Serge Knystautas <se...@lokitech.com>.
----- Original Message -----
From: "Harmeet Bedi" <ha...@kodemuse.com>


> > I'd hate to be in a situation
> > where contributors and committers have to waste time discussing minor
> > points before fixing code.
>
> I'd hate the situation too.
>
> For me at least
> - NNTP stores were not where I expected them to be.
> - NNTP protocol server was not working at all.
> - I was trying to review watchdog changes.
> - There were significant code changes that had nothing to do with proposal
> that I was spending time trying to understand.
>
> So as I found issues, posted snippets

I'm sorry Harmeet, but your posts are not coming across positively.  They
are coming off as defensive of your code, resisting change, accusatory, and
generally antagonistic.  Help is always appreciated, but at this point
you're driving Peter bonkers, and I don't think Noel is too happy either.

I'm sorry, but your posts just aren't coming across as positively as I think
you intend them.

Serge Knystautas
Loki Technologies
http://www.lokitech.com/



--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: latest checkin..

Posted by Harmeet Bedi <ha...@kodemuse.com>.
----- Original Message -----
From: "Serge Knystautas" <se...@lokitech.com>

> Harmeet Bedi wrote:
> > What did this checkin have to do with scheduler etc ?
>
> You've said this about 4 times already today, and I wish you wouldn't
> anymore.

Sorry about the repetition. Didn't realize I said it so often.

I have been mystified by the coupling.

> This is a meritocracy, and we've
> got to support that as much as possible.

Not coupling 5 things together actually tends to increase support.

It is a bit distracting to start reviewing some changes and have other
completely unrelated parts break under you.

> I'd hate to be in a situation
> where contributors and committers have to waste time discussing minor
> points before fixing code.

I'd hate the situation too.

For me at least
- NNTP stores were not where I expected them to be.
- NNTP protocol server was not working at all.
- I was trying to review watchdog changes.
- There were significant code changes that had nothing to do with proposal
that I was spending time trying to understand.

So as I found issues, posted snippets

Harmeet


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: latest checkin..

Posted by Serge Knystautas <se...@lokitech.com>.
Harmeet Bedi wrote:
> What did this checkin have to do with scheduler etc ?

You've said this about 4 times already today, and I wish you wouldn't 
anymore.  James has been dying for someone to fix issues across the 
entire server, not just the scheduler.  This is a meritocracy, and we've 
got to support that as much as possible.  I'd hate to be in a situation 
where contributors and committers have to waste time discussing minor 
points before fixing code.

-- 
Serge Knystautas
Loki Technologies
http://www.lokitech.com/


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: latest checkin..

Posted by "Peter M. Goldstein" <pe...@yahoo.com>.

Harmeet,

> > As discussed earlier, there are a number of RFC violating bugs.
This
> > was central to one of them.
> 
> Please point out where you have discussed removing article writer

>From your reply to my message:

> If these bugs can be resolved without restructuring code, you should
check
> them in.
> 
> If you need to rearchitect or refactor for these fixes, you should
post
> your
> proposal and your fixes.
>

As I've already mentioned, you seem to put removing a class in some sort
of special "design change" category.   I have no idea why.  I don't even
know what you mean by not "restructuring code", since every change is
restructuring.  In accord with our discussions, I committed my changes.
If you don't like it, -1 it.  Personally, I'm sick of the buggy NNTP
server and will be posted a vote to remove it from James shortly.

> > There was an attempt in the handler to collapse the STAT, HEAD,
BODY,
> > and ARTICLE implementations into a single implementation.  This led
> > directly to item #5 in Bug #13564.
> >
> 
> ArticleWriter did not impede this.
> One could have passed the response code to
>     private void doARTICLE(String param,ArticleWriter articleWriter)
> rather than collapse and refactor

This is ridiculous.  Why on earth would you break abstraction like this?

Aside from violating the semantic intent of the protocol - STAT, BODY,
ARTICLE, and HEAD are all separate commands, there's no actual benefit.
The code is further obfuscated (which is why there were a couple of
hidden bugs in it) and the individual messages can't be customized
without adding yet another parameter to the doARTICLE.

But more critically, we're seeing the same "that's not the way it used
to work" argument.  It didn't work before.  It works now.

And it gets rid of an extra interface and factory class.  It reduces the
semantic weight of the code.  Decouples the individual commands (those
that are not aliases of one another).

> >
> > Moreover, the ArticleWriter was conceptually flawed.  Because it
> > included protocol specific items (terminating ".", what-have-you),
> > wasn't really a writer of articles.
> 
> It is nntp protocol specific so what are your concerns about being
> protocol
> spcific ?

Specific to the protocol itself.  Not specific to the server, where it
is exposed.

> > wasn't really a writer of articles.  If it were used in a repository
> > implementation, it would corrupt all the articles in a repository.
> 
> It could write out to the client the article information. If you write
to
> source, sure you can corrupt.

So it isn't an ArticleWriter.  It doesn't write articles.  If I pass it
a FileWriter wrapped in a PrintWriter, it won't work.
 
> > Moreover, it contained no logic other than whether these protocol
> > specific items should be written - the remainder of the article
writer
> > logic is internal to the NNTPArticle implementation.
> 
> That was the intent.

Well why?

You introduced a new class to no effect.

--Peter





--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: latest checkin..

Posted by Harmeet Bedi <ha...@kodemuse.com>.
----- Original Message -----
From: "Peter M. Goldstein" <pe...@yahoo.com>
> As discussed earlier, there are a number of RFC violating bugs.  This
> was central to one of them.

Please point out where you have discussed removing article writer


>
> There was an attempt in the handler to collapse the STAT, HEAD, BODY,
> and ARTICLE implementations into a single implementation.  This led
> directly to item #5 in Bug #13564.
>

ArticleWriter did not impede this.
One could have passed the response code to
    private void doARTICLE(String param,ArticleWriter articleWriter)
rather than collapse and refactor

>
> Moreover, the ArticleWriter was conceptually flawed.  Because it
> included protocol specific items (terminating ".", what-have-you),
> wasn't really a writer of articles.

It is nntp protocol specific so what are your concerns about being protocol
spcific ?

> wasn't really a writer of articles.  If it were used in a repository
> implementation, it would corrupt all the articles in a repository.

It could write out to the client the article information. If you write to
source, sure you can corrupt.

> Moreover, it contained no logic other than whether these protocol
> specific items should be written - the remainder of the article writer
> logic is internal to the NNTPArticle implementation.

That was the intent.

What did this checkin have to do with scheduler etc ?

Harmeet


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: latest checkin..

Posted by "Peter M. Goldstein" <pe...@yahoo.com>.
Harmeet,

> Peter, why have you removed ArticleWriter and pulled code into handler
?
> 
> What was the intent and how does that relate to anything you have been
> sending around ?

As discussed earlier, there are a number of RFC violating bugs.  This
was central to one of them.

There was an attempt in the handler to collapse the STAT, HEAD, BODY,
and ARTICLE implementations into a single implementation.  This led
directly to item #5 in Bug #13564.

Moreover, the ArticleWriter was conceptually flawed.  Because it
included protocol specific items (terminating ".", what-have-you), it
wasn't really a writer of articles.  If it were used in a repository
implementation, it would corrupt all the articles in a repository.
Moreover, it contained no logic other than whether these protocol
specific items should be written - the remainder of the article writer
logic is internal to the NNTPArticle implementation.

--Peter



--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


latest checkin..

Posted by Harmeet Bedi <ha...@kodemuse.com>.
----- Original Message -----
From: <pg...@apache.org>
>   Removed:
>                src/java/org/apache/james/nntpserver ArticleWriter.java

Peter, why have you removed ArticleWriter and pulled code into handler ?

What was the intent and how does that relate to anything you have been
sending around ?

Harmeet


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>