File naming with diacritics

README.md

@@ -76,28 +76,36 @@ client = WeTransfer::Client.new(api_key: ENV.fetch('WT_API_KEY'))
 ```
 
 Now that you've got the client set up you can use  `create_transfer_and_upload_files` to, well, create a transfer, and upload all files!
+The upload of all files will be performed at the end of the block. Depending on your file sizes and network connection speed, this might take some time.
 
 ```ruby
 transfer = client.create_transfer_and_upload_files(message: 'All the Things') do |transfer|
-  # Add a file using File.open. If you do it like this, :name and :io params are optional
-  transfer.add_file(io: File.open('Gemfile'))
+  # Add files to the transfer, using add_file.
 
-  # Add a file with File.open, but give it a different name inside the transfer
-  transfer.add_file(
-    name: 'hello_world.rb',
-    io: File.open('path/to/file/with/different_name.rb')
-  )
-
-  # Using :name, :size and :io params.
+  # Making use of :name, :size and :io params.
   # Specifying the size is not very useful in this case, but feel free to explicitly
   # communicate the size of the coming io.
+  # Probably it is safer to not use the size param. But if you do use it, make sure it is correct!
   #
-  # The :name param is compulsory if it cannot be derived from the IO.
+  # The :name param is compulsory, since it cannot be derived from the IO.
   transfer.add_file(
     name: 'README.txt',
     size: 31,
     io: StringIO.new("You should read All the Things!")
   )
+
+  # Add a file with File.open, renaming the file inside the transfer.
+  # If you omit the size param, the SDK calls `size` on the io, and uses that.
+  transfer.add_file(
+    name: 'hello_world.rb',
+    io: File.open('path/to/file/with/different_name.rb')
+  )
+
+  # If you invoke add_file that has a File instance in the io param, the :name and :io params are
+  # optional.
+  # Using add_file like this will result in the SDK calling guessing the name of the io,
+  # by invoking `File.basename(io)`.
+  transfer.add_file(io: File.open('Gemfile'))
 end
 
 # To get a link to your transfer, call `url` on your transfer object:
@@ -132,9 +140,7 @@ transfer.to_h # =>
 # }
 ```
 
-The upload will be performed at the end of the block. Depending on your file sizes and network connection speed, this might take some time.
-
-What are you waiting for? Open that link in your browser! Chop chop.
+What are you waiting for? Create a transfer. Add some files. Open that link in your browser! Chop chop.
 
 If you want to have more control over which files uploads when, it is also possible.
 
@@ -227,7 +233,7 @@ Bug reports and pull requests are welcome on GitHub at <https://github.com/wetra
 
 ## License
 
-The gem is available as open source under the terms of the [MIT License](https://opensource.org/licenses/MIT) - the in-repo version of the license is [here](https://github.com/WeTransfer/wetransfer_ruby_sdk/blob/master/LICENSE.txt).
+The gem is available as open source under the terms of the [MIT License](https://opensource.org/licenses/MIT) - the in-repo version of the license is [here](https://github.com/WeTransfer/EmbedExamples/blob/master/LICENSE.txt).
 
 ## Code of Conduct
 

lib/we_transfer/transfer.rb

@@ -5,6 +5,7 @@ class Transfer
     class DuplicateFileNameError < ArgumentError; end
     class NoFilesAddedError < StandardError; end
     class FileMismatchError < StandardError; end
+    class NothingToUploadError < ArgumentError; end
 
     extend Forwardable
 
@@ -53,27 +54,38 @@ def add_file(**args)
     # Upload the file. Convenience method for uploading the parts of the file in
     # chunks. This will upload all chunks in order, single threaded.
     #
-    # @param  :name [String] The name used to add the file to the transfer
+    # @param  :id [optional, String] The id as returned when creating the transfer
     # @param  :io [optional, String] The contents to be uploaded. If the file was
     #         added (See Transfer#add_file) including an io, this can be omitted.
     #         If the file was added including an io, *and* it is included in this
     #         call, the io from this invocation will be uploaded.
     # @param :file [optional, WeTransferFile] The file instance that will be
     #         uploaded.
+    # Be aware that although all params are optional, you can only upload files
+    # if at least an id and an io are provided, of a file is provided.
     #
-    # @raise  [WeTransfer::RemoteFile::NoIoError] Will be raised if the io does
+    # @raise  [WeTransfer::Transfer::NothingToUploadError] Will be raised if
+    #         the io / id pair or a file is una
     #         not meet the minimal requirements (see MiniIo.mini_io_able?)
     # @example
     #
     # @see MiniIo.mini_io_able?
-    def upload_file(name:, io: nil, file: nil)
-      file ||= find_file(name)
-      put_io = io || file.io
+    def upload_file(id: nil, io: nil, file: nil)
+      begin
+        file ||= find_file(id)
+        put_io = io || file.io
+      rescue; end # do nothing; we raise if the expected params are missing
+      raise(
+        WeTransfer::Transfer::NothingToUploadError,
+        "Provide at least a :file, or a combination of :id and :io params."
+      ) unless (id && (io || file.io)) || file
 
+      # Do we still need next raise?
       raise(
         WeTransfer::RemoteFile::NoIoError,
-        "IO for file with name '#{name}' cannot be uploaded."
+        "IO for file with id '#{id}' cannot be uploaded."
       ) unless WeTransfer::MiniIO.mini_io_able?(put_io)
+
       (1..file.multipart.chunks).each do |chunk|
         put_url = upload_url_for_chunk(file_id: file.id, chunk: chunk)
         chunk_contents = StringIO.new(put_io.read(file.multipart.chunk_size))
@@ -90,25 +102,26 @@ def upload_file(name:, io: nil, file: nil)
     def upload_files
       files.each do |file|
         upload_file(
-          name: file.name,
+          id: file.id,
           file: file
         )
       end
       self
     end
 
-    def upload_url_for_chunk(file_id: nil, name: nil, chunk:)
-      raise ArgumentError, "missing keyword: either name or file_id is required" unless file_id || name
+    def upload_url_for_chunk(file_id: nil, chunk:)
+      raise ArgumentError, "missing keyword: either name or file_id is required" unless file_id
 
-      file_id ||= find_file(name).id
       @communicator.upload_url_for_chunk(id, file_id, chunk)
     end
 
-    def complete_file(file: nil, name: file&.name, file_id: file&.id)
-      raise ArgumentError, "missing keyword: either name or file is required" unless file || name || file_id
+    def complete_file(file: nil, name: nil, id: file&.id)
+      WeTransfer.logger.warn("[DEPRECATION]: name kw argument is deprecated") unless name.nil?
+      raise ArgumentError, "missing keyword: either name or file is required" unless file || id
+      return @communicator.complete_file(self.id, file.id, file.multipart.chunks) if file
 
-      file ||= find_file(name || file_id)
-      @communicator.complete_file(id, file.id, file.multipart.chunks)
+      amount_of_chunks = find_file(id).multipart.chunks
+      @communicator.complete_file(self.id, id, amount_of_chunks)
     end
 
     def complete_files
@@ -147,19 +160,19 @@ def to_h
 
     # Find a file inside this transfer
     #
-    # @param  [String] name_or_id, The name (as set by you) or the id (as returned)
-    #         by WeTransfer Public API of the file.
+    # @param  [String] id, The id, as returned by WeTransfer Public API, of the file.
+    #         This may also be the name of the file, which can be handy during the
+    #         create_transfer call: It allows caching without having an id yey
     # @raise  [FileMismatchError] if a file with that name or id cannot be found for
     #         this transfer
     # @return [WeTransferFile] The file you requested
     #
-    def find_file(name_or_id)
-      @found_files ||= Hash.new do |h, name_or_id|
-        h[name_or_id] = files.find { |file| [file.name, file.id].include? name_or_id }
+    def find_file(id)
+      @found_files ||= Hash.new do |h, id|
+        h[id] = files.find { |file| [file.id, file.name].include?(id) }
       end
-
-      raise FileMismatchError unless @found_files[name_or_id]
-      @found_files[name_or_id]
+      raise FileMismatchError unless @found_files[id]
+      @found_files[id]
     end
   end
 end

lib/we_transfer/we_transfer_file.rb

@@ -10,7 +10,7 @@ class WeTransferFile
     # isn't provided, but the io can provide it.
     #
     # @param  :io [optional, anything] The io of the file. This will be wrapped
-    #         in a MiniIo, or if absent in a NullMiniIo
+    #         in a MiniIo, or (if absent) wrapped in a NullMiniIo.
     # @param  :name [optional, String] The name you want to give your file. This
     #          does not have to match the original file name.
     # @param  :size [optional, Numeric] The size of the file. Has to be exact to

spec/features/client_creates_a_transfer_spec.rb

@@ -56,4 +56,44 @@
     expect(transfer.url)
       .to match %r|https://we.tl/t-|
   end
+
+  # # This test uses different transfer objects for the 4 different steps, so we isolate behavior
+  # # of all cases as much as possible
+  # it "handles diacritics in file names well", :focus do
+  #   name_with_diacritics = "file-name-with-diacritics-äëüïö.jpg".freeze
+
+  #   transfer = client.create_transfer(message: "test transfer") do |transfer|
+  #     transfer.add_file(
+  #       name: name_with_diacritics,
+  #       size: 10,
+  #       io: StringIO.new("#" * 10)
+  #     )
+  #   end
+
+  #   transfer.upload_file(name: name_with_diacritics)
+  #   transfer.complete_file(name: name_with_diacritics)
+  #   transfer.finalize
+  # end
+
+  it "handles diacritics in file names well" do
+    name_with_diacritics = "file-name-with-diacritics-äëüïö.jpg".freeze
+
+    create_transfer = client.create_transfer(message: "test transfer") do |transfer|
+      transfer.add_file(
+        name: name_with_diacritics,
+        size: 10,
+      )
+    end
+
+    file_id = create_transfer.find_file(name_with_diacritics).id
+
+    upload_file_transfer = client.find_transfer(create_transfer.id)
+    upload_file_transfer.upload_file(id: file_id, io: StringIO.new("#" * 10))
+
+    complete_file_transfer = client.find_transfer(create_transfer.id)
+    complete_file_transfer.complete_file(id: file_id)
+
+    finalize_transfer = client.find_transfer(create_transfer.id)
+    finalize_transfer.finalize
+  end
 end

spec/spec_helper.rb

@@ -27,3 +27,5 @@
 end
 
 RSpec::Mocks.configuration.verify_partial_doubles = true
+
+WeTransfer.logger = Logger.new(STDOUT)