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 >> >
