Bug: fix documentation FileTransfer

Hi.

Partly, I had a hard time understanding what those receiveFile(…) methods of IncomingFileTransfer is doing exactly.

Could you please fix the description in the following regards:

a) receiveFile(File) "throws an XMPPException when the transfer fails. This is not the case - it throws this exception if the negotiation fails

b) please clarify, that the state of a FileTransfer is only updated if using the receiveFile(File) method, not the receiveFile() method — or even better, fix this.

TIA

c) OutgoingFileTransfer.sendFile(String, long, String) sometimes returns null. This is not documented. Better throw an exception perhaps?

Seems this forum is just a black hole you can throw your writings at. Does any developer actually read this on a regular basis?

a) Well if the negotiation failes, the file transfer failes and a Exception is thrown. So the documention is not wrong. It’s just no indicator that the stream will be stable until the end of the transfer.

b) I am not sure if this is possible. The receiveFile() is sync and receiveFile(File) is async. How can the state of the sync call be updated, how do you even monitor the state? Maybe I am just not getting it. Patches are always welcome. :slight_smile:

c) If that’s so then it should be at least documented. Fixing may not be easy, if it’s not reproduceable.

Flow

a) What Flow said.

b) Flow is correct. Using this method the developer is taking responsibility for the actual transfer, so Smack does not internally track its progress since the developers own code is reading the parts.

c) Have to look into that one.

1 Like

Hi.

a) never mind. The documentation no says “Thrown if an error occurs during the file transfer negotiation process.” which is (now) correct.

b) but why not also track the process internally?!

I guess something like this would work:

--- C:\Users\Jorun\Downloads\OutgoingFileTransfer.java        2012-07-06 16:50:16 +0200
+++ source\org\jivesoftware\smackx\filetransfer\OutgoingFileTransfer.java       2012-07-06 16:42:14 +0200
@@ -112,7 +112,7 @@
         *             Thrown if an error occurs during the file transfer
         *             negotiation process.
         */
-       public synchronized OutputStream sendFile(String fileName, long fileSize,
+       public synchronized OutputStream sendFile(String fileName, final long fileSize,
                        String description) throws XMPPException {
                if (isDone() || outputStream != null) {
                        throw new IllegalStateException(
@@ -121,7 +121,70 @@
                }
                try {
                        setFileInfo(fileName, fileSize);
-                       this.outputStream = negotiateStream(fileName, fileSize, description);
+               OutputStream stream = negotiateStream(fileName, fileSize, description);
+
+               if (stream != null &&
+                   updateStatus(Status.negotiated, Status.in_progress)) {
+
+                   this.amountWritten = 0;
+                   this.outputStream = new java.io.FilterOutputStream(stream) {
+                        private void handleException(Exception e) throws IOException {
+                           OutgoingFileTransfer.this.setStatus(FileTransfer.Status.error);
+                           OutgoingFileTransfer.this.setException(e);
+                           if (e instanceof IOException) throw (IOException)e;
+                           else { IOException ioe = new IOException(); ioe.initCause(e); throw e; }
+                        }
+
+                        private void checkForCompletion() {
+                           if (OutgoingFileTransfer.this.amountWritten >= fileSize)
+                             OutgoingFileTransfer.this.updateStatus(FileTransfer.Status.in_progress,
+                               FileTransfer.Status.complete);
+                        }
+
+                        @Override
+                        public void close() throws IOException {
+                          try {
+                            out.close();
+                            OutgoingFileTransfer.this.setStatus(FileTransfer.Status.complete);
+                          } catch (Exception e) {
+                            handleException(e);
+                          }
+                        }
+
+                        @Override
+                        public void flush() throws IOException {
+                          try {
+                            out.flush();
+                          } catch (IOException e) {
+                            handleException(e);
+                          }
+                        }
+
+                        @Override
+                        public void write(byte[] b,
+                            int off,
+                            int len) throws IOException {
+                          try {
+                            out.write(b, off, len);
+                            OutgoingFileTransfer.this.amountWritten += len;
+                            checkForCompletion();
+                          } catch (Exception e) {
+                            handleException(e);
+                          }
+                        }
+
+                        @Override
+                        public void write(int b) throws IOException {
+                          try {
+                            out.write(b);
+                            OutgoingFileTransfer.this.amountWritten += 1;
+                            checkForCompletion();
+                          } catch (Exception e) {
+                            handleException(e);
+                          }
+                        }
+                     };
+                 }
                } catch (XMPPException e) {
                        handleXMPPException(e);
                        throw e;

c) I will try to get a stack trace when this happens again.

BTW, is there a publicly accessible subversion somewhere?

Jorun Gordan wrote:

BTW, is there a publicly accessible subversion somewhere?

http://fisheye.igniterealtime.org/browse/smack

Thanks, I discovered that myself.

I was looking for a real repository one could check out using a subversion client. Somewhere?

Check the source link from the downloads page.

Jorun Gordan wrote:

c) OutgoingFileTransfer.sendFile(String, long, String) sometimes returns null. This is not documented. Better throw an exception perhaps?

Looking at the code, in OutgoingFileTransfer.sendFile(String, long, long) it just does

``

this.outputStream = negotiateStream(...);
..
return outputStream;

In negotiateStream:

``

if (streamNegotiator == null) {
    setStatus(Status.error);
    setError(Error.no_response);
    return null;
}

The javadoc does not mention the possible null return. But IMO, it would be better to throw an exception.