I worked over the weekend to add another type of reply for commands like MKD, RMD, DELE etc and updated these commands. I think I should be able to check these in for review/any tweaking. Can one of you tell me if I should commit all changes together or file by file with commit comments specific to each file?
Regards, Sai Pullabhotla Phone: (402) 408-5753 Fax: (402) 408-6861 www.jMethods.com On Tue, Apr 7, 2009 at 4:11 AM, Niklas Gustavsson <[email protected]>wrote: > I think the patch looks good as a starting point, please feel free to > commit it to trunk now that you should have the powers :-) > > /niklas > > On Mon, Apr 6, 2009 at 8:00 PM, Sai Pullabhotla > <[email protected]> wrote: > > I debated about adding a new method called getPhysicalFileAsString() > which > > returns a String along with the getPhysicalFile which returns an Object. > But > > then, I thought, the PhysicalFile could do this in the toString method. > Like > > the NativeFtpFile returns java.io.File which has the string > representation > > of the file implemented in the toString(). It might still be a good idea > to > > add the AsString method to the interface so generic Ftplets (Ftplets that > > are not dependent on a specific file system used by the server such as > the > > one in the example below) can be guranteed to get some good value. > > > > The main reason why I added the getPhysicalFile is - > > > > An Ftplet implemented for audit logging purposes needs to know which file > > was downloaded, uploaded or renamed etc. Most often server administrators > > would like to see the real path instead of the virtual path (e.g. > > C:\ftpserver\user\psai\test.txt vs /test.txt) in the audit logs. If the > > FileSystem is not file based (e.g. database based), the FileSystem > > implementation could still return something like a document ID which > > uniquely identifies the file. The Ftplet can then log the physical file > > whether it is the path to the file or the document ID by calling > > FtpFile.getPhysicalFile().toString() or > FtpFile.getPhysicalFileAsString(). > > > > Sai Pullabhotla > > Phone: (402) 408-5753 > > Fax: (402) 408-6861 > > www.jMethods.com > > > > > > > > On Mon, Apr 6, 2009 at 12:32 PM, Niklas Gustavsson <[email protected] > >wrote: > > > >> 2009/4/6 David Latorre (JIRA) <[email protected]>: > >> > Are you subscribed to the developers mailing list? So we can move the > >> discussion there. > >> > >> I'm pretty sure Sai is, so let's move. > >> > >> > I hadn't thought much about these changes myself but your work looks > >> pretty good. The only thing I don't quite get is why we would use > >> FTPFile.getPhysicalFile ... Since it returns an Object, the coder > developing > >> the FTPLet should know which 'PhysicalFile' object (s)he is using. This > >> would mean in most situations that they know which FTPFile > implementation > >> they're using and hence, they could use casting to their appropriate > type. > >> And I guess it's possible to have an implementation of FTPFile that > doesn't > >> use a "PhysicalFile" object ... So what's your use case for this > addition? > >> I'm so tired i can't clearly think... > >> > >> I agree with David that the patch looks very good (I will review it a > >> bit more in detail later). I have previously argued against something > >> like getPhysicalFile, but I'm more and more leaning over to it being a > >> good idea. The reason for this is that we would encourage developers > >> to provide a standard way of getting the underlying object. But, the > >> javadocs should very clearly state that the method might return null, > >> for example if there is no reasonable object to return. > >> > >> > Another problem we have is that it is becoming quite difficult to > develop > >> pluggable components (be it an UserManager ,a Command or a ftplet) with > only > >> the API documentation. How would a user know which types of FTPReply he > >> should use if overwriting a command? - I don't have a solution for this > at > >> all. It's just something we could think about. > >> > >> That is a very interesting comment. How about we attempt to provide a > >> reasonable simple factory? It would also fit well with the OSGi > >> support. > >> > >> > Agree we should add a bug report for getUniqueFile(), using > >> createTempFile would be simpler but it is probably better to generate > an > >> unique filename and check for permission before creating the actual file > >> (so all these should be under a static lock) ; although of course > checking > >> only parent directory permissions we are good to go right now. > >> > >> Please open a JIRA and let's fix this in both trunk and the 1.0.x > branch. > >> > >> /niklas > >> > > >
