8 Replies Latest reply on Nov 28, 2013 11:08 PM by Jorun Gordan

    bug: fix documentation FileTransfer

    Jorun Gordan Bronze

      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

        • bug: fix documentation FileTransfer
          Jorun Gordan Bronze

          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?

            • Re: bug: fix documentation FileTransfer
              Flow KeyContributor

              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. :-)

               

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

               

              Flow

                • bug: fix documentation FileTransfer
                  rcollier KeyContributor

                  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.

                    • Re: bug: fix documentation FileTransfer
                      Jorun Gordan Bronze

                      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?

                  • Re: bug: fix documentation FileTransfer
                    Jorun Gordan Bronze

                    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.