David,
Some good observations :-).
In org.apache.ftpserver.ftplet.FileObject would look like this for close():
public void close() throws Exception;
In org.apache.ftpserver.ftplet.IOUtils, an extra close() would be added:
/**
* No exception <code>FileObject</code> close method.
*/
public final static void close(FileObject obj) {
if ( obj != null ) {
try {
obj.close();
} catch (Exception ex) {
// ignore on purpose
}
}
}
For those that are alphabetically-minded that close() would go just in
front of the one for InputStream [not that it really matters]. Have to
toss in some off-"The Register" humor here, no, wait that was Monty ...
In the FileObject code:
public void close() throws Exception {
...
// do some post-stream-write operations
File f = new File(fileName);
long fsize = f.getLength();
// handle any local close() exceptions here
...
}
---
The corresponding open() is the same for the ftplet entry.
public void close() throws Exception;
---
It would make sense to add a similar open() call to the commands? and
IOUtils? That way the developer always has the option to implement those
calls, err, methods. The default being an empty method in their
FileObject implementation.
---
Another question arises, does it make sense to use a specific exception
like IOException or just use the proverbial "kitchen sink" 'Exception'?
Here is why:
In one's FileObject code you could check for specific events during and
open() or close(), handling them locally. However if an error did occur,
in most cases you should still pass-up that an exception occurred so the
protocol handler(s) could inform the client. The more generic
'Exception' would make the most sense in this case as opposed to the
'IOException'.
Here is an example:
public void open() throws Exception {
// open the file and pull it from the remote
// storage, caching it locally
try {
// connect to the remote server
this.remoteServer.open(fileName)
...
} catch (IOException ex) {
throw new Exception();
} catch (FileNotFoundException ex) {
// file was not on that server, try another one
...
// file was found
...
// else file was not found on other server either
throw new Exception();
} catch (IllegalOperationException) {
// the file is a "directory"
// use Frank's fileobject as an example
LOG.error("VfsFileObject.open() - target is a directory");
throw new Exception();
}
}
Any specific errors can be handled locally, however if an error did
occur, a generic Exception is passed "up" the chain. This is more as an
FYI message to the client.
Now your earlier comment about logging errors comes in to play. Where to
log? and what to log? Or is it necessary as the developer now has the
opportunity to handle it in the local open() and close() methods?
Keep in mind that you do not have to issue an open(), it's optional, but
most times a close() is not. Having said that, there are some cases
where you may need to issue an open() but not a close. The point here is
that these are not mandatory-use methods, and do not need to be made in
order, so an open() is not required for a close().
So in summary:
1. Add a new open() and close().
2. The open() and close() will throw a generic Exception.
3. Add a new IOUtils.close() method for FileObject.
4. Add a corresponding open() in IOUtils.
5. Any specific errors can now be handled locally.
6. Is lower-level logging at the command implementation
required due to [5]?
Comments?
Andy Thomson
David M. Lloyd wrote:
Just an observation. First, anything that has a "void close() throws
IOException" method, or a covariant of such a method, should implement
java.io.Closable.
Second, a generalized static exception-eating "close" method shouldn't
eat exceptions, it should log them.
Third, that generalized static "close" method should accept a Closable
as an argument. This allows the most flexibility.
Finally - there should be one finally {} block for each closing
resource, regardless of how you eat exceptions.
So I guess you'd have:
FileObject fileObject = ...
try {
OutputStream os = ...
try {
...
} finally {
IOUtils.close(os);
}
} finally {
IOUtils.close(fileObject);
}
On 04/30/2008 11:31 AM, Andy Thomson wrote:
Frank,
The better solution is to add support for close() and open().
I added file open() and close() to the ftplet.FileObject interface.
This is to better support a virtual file system. For the native file
system this does not make too much sense, but for others it does. The
open() and close() can be thought of as an opportunity to pre-process
or post-process a stream file.
In my case I need to update some of the metadata information after the
file was written via a stream. So I added the close() in the final
statement of specific commands [see below].
From this:
finally {
IoUtils.close(os);
}
To this:
finally {
IoUtils.close(os);
file.close();
}
Then in my implementation of FileObject:
public void close() {
...
// do some post-write operations
File f = new File(fileName);
long fsize = f.getLength();
...
}
The list of commands that would need to be changed to support the
close() method are:
APPE
MD5
RETR
STOR
STOU
The classes of interest are:
ftplet:
org.apache.ftpserver.ftplet.FileObject
core:
org.apache.ftpserver.command.APPE
org.apache.ftpserver.command.MD5
org.apache.ftpserver.command.RETR
org.apache.ftpserver.command.STOR
org.apache.ftpserver.command.STOU
---
Hopefully this is useful. I made more changes to make the file system
interface(s) more "virtual", ie, support for multiple volumes and so
forth. Once I am sure all is good, I am to submit them to Niklas to
see if there was any interest in making them part of the project.
My purpose for the changes was to make the FTPSERVER file system not
care where the metadata and file content were located at. In my case
they are far apart, one is in a database, the other is somewhere else,
sometimes local, sometimes on a different storage device [like a
Centera].
I did not look at your implementation yet, so maybe the above does not
apply.
Andy
Frank van der Kleij wrote:
I am using the Mina FTPServer together with Apache Commons VFS to
provide FTP access to the Documentum CMS. I created an alternative
file system manager implementation for the FTPServer to bridge to the
VFS (more details can be found at
http://dctmvfs.sourceforge.net/ftpserver-vfs). Documentum access is
realized using a plugin for VFS
To import in Documentum I need to work with files, while the FTP (and
VFS) interfaces work with streams. I solved this by using a handler
on the close event of the stream that does the actual import. Since
all kinds of errors can occur, these errors arrive in the FTP server
on the close method on the stream, which are ignored for the moment.
FTP clients therefore think the transfer was successful.
Since I think the close is a major event in interfaces like this, I
suggest to stop ignoring the possible exceptions. For for example by
adding a close on the stream right after the method that uses the
stream and to only ignore exceptions on a close in the finally block.
The exact error message that should be sent can be a 551 as is the
current IOException handling; but perhaps there are better options,
I'm not too familiar with FTP error codes.
Does anyone have any comments?
Best regards,
Frank
Below, I've put my original JIRA request and Niklas Gustavssons comment.
Date: Sun, 6 Apr 2008 13:20:25 -0700
From: [EMAIL PROTECTED]
To: [EMAIL PROTECTED]
Subject: [jira] Commented: (FTPSERVER-119) STOR command should not
eat exceptions when closing stream
[
https://issues.apache.org/jira/browse/FTPSERVER-119?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12586175#action_12586175
]
Niklas Gustavsson commented on FTPSERVER-119:
---------------------------------------------
So, if I understand your request, you would like us to send a "551
Error on output file" error message in an exception is thrown during
close? If so, please start a discussion on the dev list
([email protected]) as it might affect other people, just to make
sure we do the right thing.
STOR command should not eat exceptions when closing stream
----------------------------------------------------------
Key: FTPSERVER-119
URL:
https://issues.apache.org/jira/browse/FTPSERVER-119
Project: FtpServer
Issue Type: Improvement
Components: Core
Affects Versions: 1.0
Environment: na
Reporter: Frank
Priority: Minor
When the STOR commands closes the outputstream, it uses
IOUtils.close(OutputStream) in a finally block which eats all
exceptions.
Wouldn't it be possible to first do a normal close on the stream
after dataConnection.transferFromClient(outStream) and only eat
exceptions on close in the finally block?
Is there a reason for always eating close exceptions?
If you consider changing it, would you also do it on branch1.4?
I'll describe my motivation below:
I have created a custom FileSystemManager based on Commons VFS. One
of the VFS plugins I use stores the output stream temporarily as a
file and then further handles the file when close is called on the
output stream.
When something goes wrong passing the file to lower levels an
exception is thrown, but the exception gets eaten by IOUtils.close
and a success message is sent to the FTP client. FileZilla for
example already shows the file being transferred since it doesn't
do a new directory listing, so for users it's quite difficult to
find failed transfers.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
_________________________________________________________________
Express yourself instantly with MSN Messenger! Download today it's FREE!
http://messenger.msn.click-url.com/go/onm00200471ave/direct/01/
