You are viewing a plain text version of this content. The canonical link for it is here.
Posted to ftpserver-users@mina.apache.org by Sai Pullabhotla <sa...@jmethods.com> on 2008/12/26 15:01:12 UTC

A couple of enhancements to consider adding

We may have talked about these a little bit, I'm posting again to see
what you guys think of the following enhancements:

Make a method available in the FtpSession which returns the session
ID. Basically, we need to maintain a session for every control
connection. This helps determine tracking of a particular session even
if simultaneous connections came from the same client machine/router.

Change the signature of Command.execute() to return some object. This
object should contain all the information about the result of the
execution. This object could be FtpResponse or a new object, Result.
This result should be made available to the Ftplets afterCommand
method so the Ftplet implementations can know all about the result of
the command. For example, a RenameResult could contain the from file,
to file, file size etc. Where as a RetrieveResult could contain the
file that was downloaded, its size and may be how long it took etc.

Let me know what you guys think of these changes.

Sai Pullabhotla
Phone: (402) 408-5753
Fax: (402) 408-6861
www.jMethods.com

Re: A couple of enhancements to consider adding

Posted by David Latorre <dv...@gmail.com>.
2009/1/1 Niklas Gustavsson <ni...@protocol7.com>

> On Wed, Dec 31, 2008 at 2:51 AM, Sai Pullabhotla
> <sa...@jmethods.com> wrote:
> > Enhance the DefaultFtpReply to hold some additional information such
> > as startTime (time when the command started executing), endTime (time
> > when the command finished executing). Ftplets may want to know this
> > information.
>
> Sounds good, this should go into the interface as well.
>
> > Change the signature of the Command.execute to return the FtpReply
> > (this may not be needed, but I think it makes more sense to do it this
> > way rather than storing the "lastReply" in the FtpSession. I
> > definitely prefer this approach.
>
> While I agree that this is cleaner, I don't think we should do this
> since it severely breaks our API. We'll do this in a future 2.0
> release I think.
>
> > Create subclasses of FtpReply where Ftplets may want to know
> > additional information about the result of a command. For example, the
> > RNTO command returns a RenameFtpReply would contain the fromFile and
> > toFile information.
>
> Right. Again, make these interfaces as well.
>
> > One thing I'm still debating on is - if we really should use FtpReply
> > objects to provide this additional information or create yet another
> > abstract class (call it Result) and call back the Ftplet afterCommand
> > method with the Result instead of the FtpReply. This Result would
> > > contain the FtpReply plus any other information. I prefer this solely
> > > based on the logical structure/naming of the objects. Both approaches
> > > would work fine, except the backward compatibility issue if we decide
> > > to go with the new Result object. To be more specific, the Result
> > > class would look like -
> >
> > I think we should add the information on the FtpReply sub-interfaces.
>
>
> Since a command can (and will) have several reply codes, I would say that
> we might probably need a distinction between the sent reply code and the
> Result of the operation, don't you think so? I'm probably missing something
> here.
>

   We could add some goodies as a isPositiveCompletion method (and such) as
well,  but I wonder if in our code FTPReply is the proper place to add these
methods.





> /niklas
>

Re: A couple of enhancements to consider adding

Posted by Sai Pullabhotla <sa...@jmethods.com>.
I started working on this and need to know what you guys think of the
following:

Instead of having a start time and end time in the FtpReply interface
- what if we have receivedTime in the FtpRequest object and sentTime
in the FtpReply. This potentially serves the same purpose - when an
Ftplet wants to know the start time, end time or total time taken by a
command.

Also, as David suggested, I would also like to add a few methods to
the FtpReply interface to indicate if the reply was positive, negative
etc.

Let me know what you think.

Regards,

Sai Pullabhotla
Phone: (402) 408-5753
Fax: (402) 408-6861
www.jMethods.com

On Thu, Jan 1, 2009 at 12:45 AM, Niklas Gustavsson <ni...@protocol7.com> wrote:
> On Wed, Dec 31, 2008 at 2:51 AM, Sai Pullabhotla
> <sa...@jmethods.com> wrote:
>> Enhance the DefaultFtpReply to hold some additional information such
>> as startTime (time when the command started executing), endTime (time
>> when the command finished executing). Ftplets may want to know this
>> information.
>
> Sounds good, this should go into the interface as well.
>
>> Change the signature of the Command.execute to return the FtpReply
>> (this may not be needed, but I think it makes more sense to do it this
>> way rather than storing the "lastReply" in the FtpSession. I
>> definitely prefer this approach.
>
> While I agree that this is cleaner, I don't think we should do this
> since it severely breaks our API. We'll do this in a future 2.0
> release I think.
>
>> Create subclasses of FtpReply where Ftplets may want to know
>> additional information about the result of a command. For example, the
>> RNTO command returns a RenameFtpReply would contain the fromFile and
>> toFile information.
>
> Right. Again, make these interfaces as well.
>
>> One thing I'm still debating on is - if we really should use FtpReply
>> objects to provide this additional information or create yet another
>> abstract class (call it Result) and call back the Ftplet afterCommand
>> method with the Result instead of the FtpReply. This Result would
>> contain the FtpReply plus any other information. I prefer this solely
>> based on the logical structure/naming of the objects. Both approaches
>> would work fine, except the backward compatibility issue if we decide
>> to go with the new Result object. To be more specific, the Result
>> class would look like -
>
>  I think we should add the information on the FtpReply sub-interfaces.
>
> /niklas
>

Re: A couple of enhancements to consider adding

Posted by Niklas Gustavsson <ni...@protocol7.com>.
On Wed, Dec 31, 2008 at 2:51 AM, Sai Pullabhotla
<sa...@jmethods.com> wrote:
> Enhance the DefaultFtpReply to hold some additional information such
> as startTime (time when the command started executing), endTime (time
> when the command finished executing). Ftplets may want to know this
> information.

Sounds good, this should go into the interface as well.

> Change the signature of the Command.execute to return the FtpReply
> (this may not be needed, but I think it makes more sense to do it this
> way rather than storing the "lastReply" in the FtpSession. I
> definitely prefer this approach.

While I agree that this is cleaner, I don't think we should do this
since it severely breaks our API. We'll do this in a future 2.0
release I think.

> Create subclasses of FtpReply where Ftplets may want to know
> additional information about the result of a command. For example, the
> RNTO command returns a RenameFtpReply would contain the fromFile and
> toFile information.

Right. Again, make these interfaces as well.

> One thing I'm still debating on is - if we really should use FtpReply
> objects to provide this additional information or create yet another
> abstract class (call it Result) and call back the Ftplet afterCommand
> method with the Result instead of the FtpReply. This Result would
> contain the FtpReply plus any other information. I prefer this solely
> based on the logical structure/naming of the objects. Both approaches
> would work fine, except the backward compatibility issue if we decide
> to go with the new Result object. To be more specific, the Result
> class would look like -

 I think we should add the information on the FtpReply sub-interfaces.

/niklas

Re: A couple of enhancements to consider adding

Posted by Sai Pullabhotla <sa...@jmethods.com>.
Niklas,

I'm planning on working on the issue FTPSERVER-253
(http://issues.apache.org/jira/browse/FTPSERVER-253) over this weekend
and want to make sure we are on the same page.

Essentially below is what I'm planning on doing:

Enhance the DefaultFtpReply to hold some additional information such
as startTime (time when the command started executing), endTime (time
when the command finished executing). Ftplets may want to know this
information.

Change the signature of the Command.execute to return the FtpReply
(this may not be needed, but I think it makes more sense to do it this
way rather than storing the "lastReply" in the FtpSession. I
definitely prefer this approach.

Create subclasses of FtpReply where Ftplets may want to know
additional information about the result of a command. For example, the
RNTO command returns a RenameFtpReply would contain the fromFile and
toFile information.

I don't think we will have too many subclasses of the DefaultFtpReply,
but I will see what I can do when I dig into the code.

One thing I'm still debating on is - if we really should use FtpReply
objects to provide this additional information or create yet another
abstract class (call it Result) and call back the Ftplet afterCommand
method with the Result instead of the FtpReply. This Result would
contain the FtpReply plus any other information. I prefer this solely
based on the logical structure/naming of the objects. Both approaches
would work fine, except the backward compatibility issue if we decide
to go with the new Result object. To be more specific, the Result
class would look like -

public abstract class Result {

    private FtpReply reply;
    private boolean success;
    private long startTime;
    private long endTime;
}

A sub class of Result, RenameResult would look like -

public abstract class RenameResult extends Result {
    private FtpFile from;
    private FtpFile to;
}

I will let you call on if we should just keep everything in the
FtpReply vs the new Result class.

Regards,

Sai Pullabhotla
Phone: (402) 408-5753
Fax: (402) 408-6861
www.jMethods.com




On Fri, Dec 26, 2008 at 1:03 PM, Niklas Gustavsson <ni...@protocol7.com> wrote:
> On Fri, Dec 26, 2008 at 5:26 PM, Sai Pullabhotla
> <sa...@jmethods.com> wrote:
>> The purpose of the session ID is just for logging purposes. Let us say
>> I open two simultaneous connections to the FtpServer from my PC both
>> using the same user name, and do several file operations. While we
>> have the remote client's information such as IP address, there is
>> really no way to tell what command was executed from what session.
>> Providing a session ID (which could be the one from Mina) will help
>> associate an action with a specific session. The server administrator
>> can see the exact sequence of commands that were executed in a
>> particular session. Hope this makes sense.
>
> I see, that would be a simple improvement, feel free to open a JIRA
> issue and I'll fix it right away.
>
>> I will be glad to take up the command/reply related changes I
>> mentioned. Will submit a patch soon. I will open a JIRA for this.
>
> Great!
>
> /niklas
>

Re: A couple of enhancements to consider adding

Posted by Niklas Gustavsson <ni...@protocol7.com>.
On Fri, Dec 26, 2008 at 5:26 PM, Sai Pullabhotla
<sa...@jmethods.com> wrote:
> The purpose of the session ID is just for logging purposes. Let us say
> I open two simultaneous connections to the FtpServer from my PC both
> using the same user name, and do several file operations. While we
> have the remote client's information such as IP address, there is
> really no way to tell what command was executed from what session.
> Providing a session ID (which could be the one from Mina) will help
> associate an action with a specific session. The server administrator
> can see the exact sequence of commands that were executed in a
> particular session. Hope this makes sense.

I see, that would be a simple improvement, feel free to open a JIRA
issue and I'll fix it right away.

> I will be glad to take up the command/reply related changes I
> mentioned. Will submit a patch soon. I will open a JIRA for this.

Great!

/niklas

Re: A couple of enhancements to consider adding

Posted by Sai Pullabhotla <sa...@jmethods.com>.
The purpose of the session ID is just for logging purposes. Let us say
I open two simultaneous connections to the FtpServer from my PC both
using the same user name, and do several file operations. While we
have the remote client's information such as IP address, there is
really no way to tell what command was executed from what session.
Providing a session ID (which could be the one from Mina) will help
associate an action with a specific session. The server administrator
can see the exact sequence of commands that were executed in a
particular session. Hope this makes sense.

I will be glad to take up the command/reply related changes I
mentioned. Will submit a patch soon. I will open a JIRA for this.

Sai Pullabhotla
Phone: (402) 408-5753
Fax: (402) 408-6861
www.jMethods.com




On Fri, Dec 26, 2008 at 10:01 AM, Niklas Gustavsson
<ni...@protocol7.com> wrote:
> On Fri, Dec 26, 2008 at 3:01 PM, Sai Pullabhotla
> <sa...@jmethods.com> wrote:
>> Make a method available in the FtpSession which returns the session
>> ID. Basically, we need to maintain a session for every control
>> connection. This helps determine tracking of a particular session even
>> if simultaneous connections came from the same client machine/router.
>
> Could you please explain this further? We already maintain a session
> for each connection (well, MINA does it for us). It does not have a
> session ID, but I'm not sure I see the need for it?
>
>> Change the signature of Command.execute() to return some object. This
>> object should contain all the information about the result of the
>> execution. This object could be FtpResponse or a new object, Result.
>> This result should be made available to the Ftplets afterCommand
>> method so the Ftplet implementations can know all about the result of
>> the command. For example, a RenameResult could contain the from file,
>> to file, file size etc. Where as a RetrieveResult could contain the
>> file that was downloaded, its size and may be how long it took etc.
>
> As you're aware, we already pass the FtpReply to Ftplets. I agree that
> we should enhance this by providing subclasses of FtpReply that
> provide additional details on the result, like the renamed file and so
> on. I would like to do this soon after 1.0. If you want to hack away
> at this, feel very welcome :-)
>
> /niklas
>

Re: A couple of enhancements to consider adding

Posted by Niklas Gustavsson <ni...@protocol7.com>.
On Fri, Dec 26, 2008 at 3:01 PM, Sai Pullabhotla
<sa...@jmethods.com> wrote:
> Make a method available in the FtpSession which returns the session
> ID. Basically, we need to maintain a session for every control
> connection. This helps determine tracking of a particular session even
> if simultaneous connections came from the same client machine/router.

Could you please explain this further? We already maintain a session
for each connection (well, MINA does it for us). It does not have a
session ID, but I'm not sure I see the need for it?

> Change the signature of Command.execute() to return some object. This
> object should contain all the information about the result of the
> execution. This object could be FtpResponse or a new object, Result.
> This result should be made available to the Ftplets afterCommand
> method so the Ftplet implementations can know all about the result of
> the command. For example, a RenameResult could contain the from file,
> to file, file size etc. Where as a RetrieveResult could contain the
> file that was downloaded, its size and may be how long it took etc.

As you're aware, we already pass the FtpReply to Ftplets. I agree that
we should enhance this by providing subclasses of FtpReply that
provide additional details on the result, like the renamed file and so
on. I would like to do this soon after 1.0. If you want to hack away
at this, feel very welcome :-)

/niklas