novoda / download-manager

A library that handles long-running downloads, handling the network interactions and retrying downloads automatically after failures
Apache License 2.0
483 stars 63 forks source link

Normalise paths when building BatchFile #480

Closed frapontillo closed 5 years ago

frapontillo commented 5 years ago

Problem

When building a BatchFile with LiteBatchFileBuilder, we were simply checking that path wasn't the file separator /, so we wouldn't add a duplicated separator after that. But that fix (#475) didn't work for cases where the path is something like my/path/to/the/file/ (with a file separator at the end), so we were essentially still adding a double /, causing issues like failed downloads.

Solution

The solution consists in treating each string we need to concatenate as the same kind of path segment:

  1. If the first segment (storageRoot.path()) starts with a file separator, we add a file separator to the StringBuilder
  2. Then, for each element
    1. If the element is empty, we ignore it
    2. We add a file separator to the StringBuilder if the last char of the StringBuilder is not a file separator
    3. We remove any leading or trailing file separators (/) from the segment
    4. We add the segment to the StringBuilder

Tests

Tests have been added to prove that all edge and middle cases are covered for.

frapontillo commented 5 years ago

Implementation changed in 0224efaef89fd01b8e8c951efaeff7cf3cf4b5da to clean up internal duplicated separators too 💯.

frapontillo commented 5 years ago

We leave the developer to specify the file path as best as possible. This would only provide some safety around what's been passed, which should be separated with a File.separator.