Merge pull request #958 from rantianhua/master

fix: data lose when retry
lingochamp · Mar 11, 2018 · 0795595 · 0795595
2 parents 09cce32 + 3bb26a4
commit 0795595
Show file tree

Hide file tree

Showing 9 changed files with 68 additions and 25 deletions.
diff --git a/library/src/main/java/com/liulishuo/filedownloader/database/RemitDatabase.java b/library/src/main/java/com/liulishuo/filedownloader/database/RemitDatabase.java
@@ -93,6 +93,10 @@ private void syncCacheToDB(int id) {
             FileDownloadLog.d(this, "sync cache to db %d", id);
         }
 
+        // need to attention that the `sofar` in `FileDownloadModel` database will
+        // be updated even through this is a multiple connections task. But the
+        // multi-connections task only concern about the `ConnectionModel` database,
+        // so it doesn't matter in current.
         realDatabase.update(cachedDatabase.find(id));
         final List<ConnectionModel> modelList = cachedDatabase.findConnectionModel(id);
         realDatabase.removeConnections(id);

diff --git a/library/src/main/java/com/liulishuo/filedownloader/download/ConnectTask.java b/library/src/main/java/com/liulishuo/filedownloader/download/ConnectTask.java
@@ -57,6 +57,23 @@ private ConnectTask(ConnectionProfile profile,
         this.profile = profile;
     }
 
+    void updateConnectionProfile(long downloadedOffset) {
+        if (downloadedOffset == profile.currentOffset) {
+            FileDownloadLog.w(this, "no data download, no need to update");
+            return;
+        }
+        final long newContentLength =
+                profile.contentLength - (downloadedOffset - profile.currentOffset);
+        profile = ConnectionProfile.ConnectionProfileBuild.buildConnectionProfile(
+                profile.startOffset,
+                downloadedOffset,
+                profile.endOffset,
+                newContentLength);
+        if (FileDownloadLog.NEED_LOG) {
+            FileDownloadLog.i(this, "after update profile:%s", profile);
+        }
+    }
+
     FileDownloadConnection connect() throws IOException, IllegalAccessException {
         FileDownloadConnection connection = CustomComponentHolder.getImpl().createConnection(url);
 

diff --git a/library/src/main/java/com/liulishuo/filedownloader/download/DownloadLaunchRunnable.java b/library/src/main/java/com/liulishuo/filedownloader/download/DownloadLaunchRunnable.java
@@ -308,7 +308,7 @@ public void run() {
                         | InterruptedException | IllegalArgumentException
                         | FileDownloadGiveUpRetryException e) {
                     if (isRetry(e)) {
-                        onRetry(e, 0);
+                        onRetry(e);
                         continue;
                     } else {
                         onError(e);
@@ -791,7 +791,7 @@ public void onCompleted(DownloadRunnable doneRunnable, long startOffset, long en
             return;
         }
 
-        final int doneConnectionIndex = doneRunnable == null ? -1 : doneRunnable.connectionIndex;
+        final int doneConnectionIndex = doneRunnable.connectionIndex;
         if (FileDownloadLog.NEED_LOG) {
             FileDownloadLog.d(this, "the connection has been completed(%d): [%d, %d)  %d",
                     doneConnectionIndex, startOffset, endOffset, model.getTotal());
@@ -854,7 +854,7 @@ public void onError(Exception exception) {
     }
 
     @Override
-    public void onRetry(Exception exception, long invalidIncreaseBytes) {
+    public void onRetry(Exception exception) {
         if (paused) {
             if (FileDownloadLog.NEED_LOG) {
                 FileDownloadLog.d(this, "the task[%d] has already been paused, so pass the"
@@ -868,7 +868,7 @@ public void onRetry(Exception exception, long invalidIncreaseBytes) {
                     validRetryTimes, model.getId());
         }
 
-        statusCallback.onRetry(exception, validRetryTimes, invalidIncreaseBytes);
+        statusCallback.onRetry(exception, validRetryTimes);
     }
 
     @Override

diff --git a/library/src/main/java/com/liulishuo/filedownloader/download/DownloadRunnable.java b/library/src/main/java/com/liulishuo/filedownloader/download/DownloadRunnable.java
@@ -19,14 +19,18 @@
 import android.os.Process;
 
 import com.liulishuo.filedownloader.connection.FileDownloadConnection;
+import com.liulishuo.filedownloader.database.FileDownloadDatabase;
 import com.liulishuo.filedownloader.exception.FileDownloadGiveUpRetryException;
+import com.liulishuo.filedownloader.model.ConnectionModel;
 import com.liulishuo.filedownloader.model.FileDownloadHeader;
+import com.liulishuo.filedownloader.model.FileDownloadModel;
 import com.liulishuo.filedownloader.util.FileDownloadLog;
 import com.liulishuo.filedownloader.util.FileDownloadUtils;
 
 import java.io.IOException;
 import java.net.HttpURLConnection;
 import java.net.SocketException;
+import java.util.List;
 
 /**
  * The single download runnable used for establish one connection and fetch data from it.
@@ -122,20 +126,22 @@ public void run() {
             } catch (IllegalAccessException | IOException | FileDownloadGiveUpRetryException
                     | IllegalArgumentException e) {
                 if (callback.isRetry(e)) {
-                    if (!isConnected) {
-                        callback.onRetry(e, 0);
-                    } else if (fetchDataTask != null) {
-                        // connected
-                        final long invalidIncreaseBytes = fetchDataTask.currentOffset - beginOffset;
-                        callback.onRetry(e, invalidIncreaseBytes);
-                    } else {
+                    if (isConnected && fetchDataTask == null) {
                         // connected but create fetch data task failed, give up directly.
                         FileDownloadLog.w(this, "it is valid to retry and connection is valid but"
                                 + " create fetch-data-task failed, so give up directly with %s", e);
                         callback.onError(e);
                         break;
+                    } else {
+                        if (fetchDataTask != null) {
+                            //update currentOffset in ConnectionProfile
+                            final long downloadedOffset = getDownloadedOffset();
+                            if (downloadedOffset > 0) {
+                                connectTask.updateConnectionProfile(downloadedOffset);
+                            }
+                        }
+                        callback.onRetry(e);
                     }
-
                 } else {
                     callback.onError(e);
                     break;
@@ -148,6 +154,24 @@ public void run() {
 
     }
 
+    private long getDownloadedOffset() {
+        final FileDownloadDatabase database = CustomComponentHolder.getImpl().getDatabaseInstance();
+        if (connectionIndex >= 0) {
+            // is multi connection
+            List<ConnectionModel> connectionModels = database.findConnectionModel(downloadId);
+            for (ConnectionModel connectionModel : connectionModels) {
+                if (connectionModel.getIndex() == connectionIndex) {
+                    return connectionModel.getCurrentOffset();
+                }
+            }
+        } else {
+            // is single connection
+            FileDownloadModel downloadModel = database.find(downloadId);
+            return downloadModel.getSoFar();
+        }
+        return 0;
+    }
+
     public static class Builder {
         private final ConnectTask.Builder connectTaskBuilder = new ConnectTask.Builder();
         private ProcessCallback callback;

diff --git a/library/src/main/java/com/liulishuo/filedownloader/download/DownloadStatusCallback.java b/library/src/main/java/com/liulishuo/filedownloader/download/DownloadStatusCallback.java
@@ -167,9 +167,8 @@ void onProgress(long increaseBytes) {
         }
     }
 
-    void onRetry(Exception exception, int remainRetryTimes, long invalidIncreaseBytes) {
+    void onRetry(Exception exception, int remainRetryTimes) {
         this.callbackIncreaseBuffer.set(0);
-        model.increaseSoFar(-invalidIncreaseBytes);
 
         if (handler == null) {
             // direct

diff --git a/library/src/main/java/com/liulishuo/filedownloader/download/FetchDataTask.java b/library/src/main/java/com/liulishuo/filedownloader/download/FetchDataTask.java
@@ -93,7 +93,7 @@ public void run() throws IOException, IllegalAccessException, IllegalArgumentExc
 
         if (this.contentLength > 0 && contentLength != this.contentLength) {
             final String range;
-            if (endOffset == 0) {
+            if (endOffset == ConnectionProfile.RANGE_INFINITE) {
                 range = FileDownloadUtils.formatString("range[%d-)", currentOffset);
             } else {
                 range = FileDownloadUtils.formatString("range[%d-%d)", currentOffset, endOffset);
@@ -186,7 +186,7 @@ public void run() throws IOException, IllegalAccessException, IllegalArgumentExc
         if (contentLength != TOTAL_VALUE_IN_CHUNKED_RESOURCE && contentLength != fetchedLength) {
             throw new FileDownloadGiveUpRetryException(
                     FileDownloadUtils.formatString("fetched length[%d] != content length[%d],"
-                                    + " range[%d, %d) offset[%d] fetch begin offset",
+                                    + " range[%d, %d) offset[%d] fetch begin offset[%d]",
                             fetchedLength, contentLength,
                             startOffset, endOffset, currentOffset, fetchBeginOffset));
         }
@@ -229,7 +229,7 @@ private void sync() {
         }
 
         if (bufferPersistToDevice) {
-            final boolean isBelongMultiConnection = hostRunnable != null;
+            final boolean isBelongMultiConnection = connectionIndex >= 0;
             if (isBelongMultiConnection) {
                 // only need update the connection table.
                 database.updateConnectionModel(downloadId, connectionIndex, currentOffset);

diff --git a/library/src/main/java/com/liulishuo/filedownloader/download/ProcessCallback.java b/library/src/main/java/com/liulishuo/filedownloader/download/ProcessCallback.java
@@ -29,7 +29,7 @@ public interface ProcessCallback {
 
     void onError(Exception exception);
 
-    void onRetry(Exception exception, long invalidIncreaseBytes);
+    void onRetry(Exception exception);
 
     void syncProgressFromCache();
 }
diff --git a/library/src/test/java/com/liulishuo/filedownloader/download/DownloadLaunchRunnableTest.java b/library/src/test/java/com/liulishuo/filedownloader/download/DownloadLaunchRunnableTest.java
@@ -58,11 +58,11 @@ public void onRetry_validRetryTimesDecrease_only1() {
                         mock(IThreadPoolMonitor.class),
                         1000, 100, false,
                         true, 3);
-        launchRunnable.onRetry(mock(Exception.class), 0);
+        launchRunnable.onRetry(mock(Exception.class));
         assertThat(launchRunnable.validRetryTimes).isEqualTo(2);
-        launchRunnable.onRetry(mock(Exception.class), 0);
+        launchRunnable.onRetry(mock(Exception.class));
         assertThat(launchRunnable.validRetryTimes).isEqualTo(1);
-        launchRunnable.onRetry(mock(Exception.class), 0);
+        launchRunnable.onRetry(mock(Exception.class));
         assertThat(launchRunnable.validRetryTimes).isEqualTo(0);
     }
 

diff --git a/library/src/test/java/com/liulishuo/filedownloader/download/DownloadRunnableTest.java b/library/src/test/java/com/liulishuo/filedownloader/download/DownloadRunnableTest.java
@@ -28,7 +28,6 @@
 import java.net.HttpURLConnection;
 
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
@@ -67,7 +66,7 @@ public void run_withConnectFailed_retry() throws IOException, IllegalAccessExcep
 
         downloadRunnable.run();
 
-        verify(mockCallback).onRetry(mockIOException, 0);
+        verify(mockCallback).onRetry(mockIOException);
     }
 
     @Test
@@ -79,7 +78,7 @@ public void run_responseCodeNotMet_error() throws IOException, IllegalAccessExce
         downloadRunnable.run();
 
         // retry first.
-        verify(mockCallback).onRetry(any(Exception.class), anyLong());
+        verify(mockCallback).onRetry(any(Exception.class));
 
         // then callback error.
         verify(mockCallback).onError(any(Exception.class));
@@ -113,7 +112,7 @@ public void onError(Exception exception) {
 
 
         @Override
-        public void onRetry(Exception exception, long invalidIncreaseBytes) {
+        public void onRetry(Exception exception) {
         }
 
         @Override