[supply] introduce a new synchronization logic for screenshots (#21498)

* [supply] Implement screenshots upload sync

Compare the checksum of the images to be uploaded with the ones already in remote, and skip upload if they already match

* Add unit tests

* Improve YARD doc

* fix typo in comment

* Apply suggestions from code review

Co-authored-by: Roger Oba <rogerluan.oba@gmail.com>

* Remove redundant condition

* Add assertion about not deleting _all_ screenshots when `sync_image_upload` is set

---------

Co-authored-by: Roger Oba <rogerluan.oba@gmail.com>

supply/lib/supply.rb

@@ -3,6 +3,7 @@
 require 'supply/client'
 require 'supply/listing'
 require 'supply/apk_listing'
+require 'supply/image_listing'
 require 'supply/generated_universal_apk'
 require 'supply/release_listing'
 require 'supply/uploader'

supply/lib/supply/client.rb

@@ -532,6 +532,12 @@ def update_obb(apk_version_code, expansion_file_type, references_version, file_s
     # @!group Screenshots
     #####################################################
 
+    # Fetch all the images of a given type and for a given language of your store listing
+    #
+    # @param [String] image_type Typically one of the elements of either Supply::IMAGES_TYPES or Supply::SCREENSHOT_TYPES
+    # @param [String] language Localization code (a BCP-47 language tag; for example, "de-AT" for  Austrian German).
+    # @return [Array<ImageListing>] A list of ImageListing instances describing each image with its id, sha256 and url
+    #
     def fetch_images(image_type: nil, language: nil)
       ensure_active_edit!
 
@@ -544,29 +550,17 @@ def fetch_images(image_type: nil, language: nil)
         )
       end
 
-      urls = (result.images || []).map(&:url)
-      images = urls.map do |url|
-        uri = URI.parse(url)
-        clean_url = [
-          uri.scheme,
-          uri.userinfo,
-          uri.host,
-          uri.port,
-          uri.path
-        ].join
-
-        UI.verbose("Initial URL received: '#{url}'")
-        UI.verbose("Removed params ('#{uri.query}') from the URL")
-        UI.verbose("URL after removing params: '#{clean_url}'")
-
-        full_url = "#{url}=s0" # '=s0' param ensures full image size is returned (https://github.com/fastlane/fastlane/pull/14322#issuecomment-473012462)
-        full_url
+      (result.images || []).map do |row|
+        full_url = "#{row.url}=s0" # '=s0' param ensures full image size is returned (https://github.com/fastlane/fastlane/pull/14322#issuecomment-473012462)
+        ImageListing.new(row.id, row.sha1, row.sha256, full_url)
       end
-
-      return images
     end
 
-    # @param image_type (e.g. phoneScreenshots, sevenInchScreenshots, ...)
+    # Upload an image or screenshot of a specific type for a given language to your store listing
+    #
+    # @param [String] image_type (e.g. phoneScreenshots, sevenInchScreenshots, ...)
+    # @param [String] language localization code (i.e. BCP-47 language tag as in `pt-BR`)
+    #
     def upload_image(image_path: nil, image_type: nil, language: nil)
       ensure_active_edit!
 
@@ -582,6 +576,11 @@ def upload_image(image_path: nil, image_type: nil, language: nil)
       end
     end
 
+    # Remove all screenshots of a given image_type and language from the store listing
+    #
+    # @param [String] image_type (e.g. phoneScreenshots, sevenInchScreenshots, ...)
+    # @param [String] language localization code (i.e. BCP-47 language tag as in `pt-BR`)
+    #
     def clear_screenshots(image_type: nil, language: nil)
       ensure_active_edit!
 
@@ -595,6 +594,26 @@ def clear_screenshots(image_type: nil, language: nil)
       end
     end
 
+    # Remove a specific screenshot of a given image_type and language from the store listing
+    #
+    # @param [String] image_type (e.g. phoneScreenshots, sevenInchScreenshots, ...)
+    # @param [String] language localization code (i.e. BCP-47 language tag as in `pt-BR`)
+    # @param [String] image_id The id of the screenshot to remove (as per `ImageListing#id`)
+    #
+    def clear_screenshot(image_type: nil, language: nil, image_id: nil)
+      ensure_active_edit!
+
+      call_google_api do
+        client.delete_edit_image(
+          current_package_name,
+          current_edit.id,
+          language,
+          image_type,
+          image_id
+        )
+      end
+    end
+
     def upload_obb(obb_file_path: nil, apk_version_code: nil, expansion_file_type: nil)
       ensure_active_edit!
 

supply/lib/supply/image_listing.rb

@@ -0,0 +1,15 @@
+module Supply
+  class ImageListing
+    attr_reader :id
+    attr_reader :sha1
+    attr_reader :sha256
+    attr_reader :url
+
+    def initialize(id, sha1, sha256, url)
+      @id = id
+      @sha1 = sha1
+      @sha256 = sha256
+      @url = url
+    end
+  end
+end

supply/lib/supply/options.rb

@@ -210,6 +210,11 @@ def self.available_options
                                      description: "Whether to skip uploading SCREENSHOTS",
                                      type: Boolean,
                                      default_value: false),
+        FastlaneCore::ConfigItem.new(key: :sync_image_upload,
+                                     env_name: "SUPPLY_SYNC_IMAGE_UPLOAD",
+                                     description: "Whether to use sha256 comparison to skip upload of images and screenshots that are already in Play Store",
+                                     type: Boolean,
+                                     default_value: false),
         FastlaneCore::ConfigItem.new(key: :track_promote_to,
                                      env_name: "SUPPLY_TRACK_PROMOTE_TO",
                                      optional: true,

supply/lib/supply/setup.rb

@@ -66,7 +66,7 @@ def download_images(listing)
 
           UI.message("Downloading `#{image_type}` for #{listing.language}...")
 
-          urls = client.fetch_images(image_type: image_type, language: listing.language)
+          urls = client.fetch_images(image_type: image_type, language: listing.language).map(&:url)
           next if urls.nil? || urls.empty?
 
           image_counter = 1 # Used to prefix the downloaded files, so order is preserved.

supply/lib/supply/uploader.rb

@@ -270,7 +270,17 @@ def upload_images(language)
         path = Dir.glob(search, File::FNM_CASEFOLD).last
         next unless path
 
-        UI.message("Uploading image file #{path}...")
+        if Supply.config[:sync_image_upload]
+          UI.message("🔍 Checking #{image_type} checksum...")
+          existing_images = client.fetch_images(image_type: image_type, language: language)
+          sha256 = Digest::SHA256.file(path).hexdigest
+          if existing_images.map(&:sha256).include?(sha256)
+            UI.message("🟰 Skipping upload of screenshot #{path} as remote sha256 matches.")
+            next
+          end
+        end
+
+        UI.message("⬆️ Uploading image file #{path}...")
         client.upload_image(image_path: File.expand_path(path),
                             image_type: image_type,
                               language: language)
@@ -280,13 +290,30 @@ def upload_images(language)
     def upload_screenshots(language)
       Supply::SCREENSHOT_TYPES.each do |screenshot_type|
         search = File.join(metadata_path, language, Supply::IMAGES_FOLDER_NAME, screenshot_type, "*.#{IMAGE_FILE_EXTENSIONS}")
-        paths = Dir.glob(search, File::FNM_CASEFOLD)
+        paths = Dir.glob(search, File::FNM_CASEFOLD).sort
         next unless paths.count > 0
 
-        client.clear_screenshots(image_type: screenshot_type, language: language)
+        if Supply.config[:sync_image_upload]
+          UI.message("🔍 Checking #{screenshot_type} checksums...")
+          existing_images = client.fetch_images(image_type: screenshot_type, language: language)
+          # Don't keep images that either don't exist locally, or that are out of order compared to the `paths` to upload
+          first_path_checksum = Digest::SHA256.file(paths.first).hexdigest
+          existing_images.each do |image|
+            if image.sha256 == first_path_checksum
+              UI.message("🟰 Skipping upload of screenshot #{paths.first} as remote sha256 matches.")
+              paths.shift # Remove first path from the list of paths to be uploaded
+              first_path_checksum = paths.empty? ? nil : Digest::SHA256.file(paths.first).hexdigest
+            else
+              UI.message("🚮 Deleting #{language} screenshot id ##{image.id} as it does not exist locally or is out of order...")
+              client.clear_screenshot(image_type: screenshot_type, language: language, image_id: image.id)
+            end
+          end
+        else
+          client.clear_screenshots(image_type: screenshot_type, language: language)
+        end
 
-        paths.sort.each do |path|
-          UI.message("Uploading screenshot #{path}...")
+        paths.each do |path|
+          UI.message("⬆️  Uploading screenshot #{path}...")
           client.upload_image(image_path: File.expand_path(path),
                               image_type: screenshot_type,
                                 language: language)

supply/spec/client_spec.rb

@@ -54,6 +54,7 @@
         expect(subject.class.method_defined?(:list_edit_images)).to eq(true)
         expect(subject.class.method_defined?(:upload_edit_image)).to eq(true)
         expect(subject.class.method_defined?(:deleteall_edit_image)).to eq(true)
+        expect(subject.class.method_defined?(:delete_edit_image)).to eq(true)
         expect(subject.class.method_defined?(:upload_edit_expansionfile)).to eq(true)
         expect(subject.class.method_defined?(:uploadapk_internalappsharingartifact)).to eq(true)
         expect(subject.class.method_defined?(:uploadbundle_internalappsharingartifact)).to eq(true)