On Sun, Nov 16, 2008 at 4:09 PM, Sai Pullabhotla
<[EMAIL PROTECTED]> wrote:
> I started changing the code the way we talked and found the following:
>
> The only common place where all FtpReplies go out seems to be from the
> FtpIOSession.writeObject() method. So, I updated this method to store
> the last reply in an instance variable of FtpIOSession class. Below is
> the new writeObject method:
>
> public WriteFuture write(Object message) {
> WriteFuture future = wrappedSession.write(message);
> this.lastReply = (FtpReply) message;
> return future;
> }
>
>
> Then in the DefaultFtpHandler, line 144ish, added the new parameter on
> the call to ftplet.afterCommand.
>
> try {
> ftpletRet =
> ftplets.afterCommand(session.getFtpletSession(),
> request, session.getLastReply());
> } catch (Exception e) {
> LOG.debug("Ftplet container threw exception", e);
> ftpletRet = FtpletResult.DISCONNECT;
> }
As noted before, multiple replies can be returned on one command (e.g.
during a STOR), thus the patch needs support for this. Also, it will
need unit tests for all functionality.
> So, the question to you is, do we really want to change the signature
> of Ftplet.afterCommand to include the FtpReply parameter or just have
> the Ftplet implementations retrieve the last reply from the
> FtpSession/FtpIoSession?
I would like it in the Ftplet callback rather than the session.
> Also, for passing additional information about the result of a command
> (as discussed in my previous email), I was thinking we should have a
> return value from the execute() method of Command class. This return
> type could be a ExecutionResult object which can be
> implemented/overridden by various commands to provide results with
> different parameters. If you like this idea, then need to update the
> Ftplet afterCommand method to simply include the ExecutionResult
> parameter which tells callers all about the result.
>
> In summary, I think we need to change the
> void execute(FtpIoSession, FtpServerContext, FtpRequest)
> to
> ExecutionResult execute(FtpIoSession, FtpServerContext, FtpRequest)
Let's wait with this patch, I don't think the change in the interface
is necessary and we can hold this until after 1.0.
/niklas
