refactor git

app/models/communication/website/git_file.rb

@@ -34,29 +34,6 @@ class Communication::Website::GitFile < ApplicationRecord
     website.git_repository.add_git_file git_file
   end
 
-  def should_create?
-    !should_destroy? &&
-    !exists_on_git? &&
-    (
-      !synchronized_with_git? ||
-      previous_path.nil? ||
-      previous_sha.nil?
-    )
-  end
-
-  def should_update?
-    !should_destroy? &&
-    (
-      different_path? ||
-      different_sha?
-    )
-  end
-
-  def should_destroy?
-    will_be_destroyed ||
-    path.nil?
-  end
-
   def path
     @path ||= about.git_path(website)&.gsub(/\/+/, '/')
   end
@@ -87,31 +64,4 @@ class Communication::Website::GitFile < ApplicationRecord
   def git_sha
     @git_sha ||= git_sha_for(path)
   end
-
-  # Based on content, with the provider's algorithm (sha1 or sha256)
-  def computed_sha
-    @computed_sha ||= website.git_repository.computed_sha to_s
-  end
-
-  def exists_on_git?
-    previous_git_sha.present? || # The file exists where it was last time
-    (
-      previous_path.nil? && # Never saved in the database
-      git_sha.present?      # but it exists in the git repo
-    )
-  end
-
-  def synchronized_with_git?
-    exists_on_git? && # File exists
-    previous_path == path && # at the same place
-    previous_git_sha == previous_sha # with the same content
-  end
-
-  def different_path?
-    previous_path != path
-  end
-
-  def different_sha?
-    previous_sha != computed_sha
-  end
 end

app/services/git/providers/abstract.rb

@@ -11,6 +11,30 @@ class Git::Providers::Abstract
     repository.present? && access_token.present?
   end
 
+  def create_file(path, content)
+    raise NotImplementedError
+  end
+
+  def update_file(path, previous_path, content)
+    raise NotImplementedError
+  end
+
+  def destroy_file(path)
+    raise NotImplementedError
+  end
+
+  def push(commit_message)
+    raise NotImplementedError
+  end
+
+  def computed_sha(string)
+    raise NotImplementedError
+  end
+
+  def git_sha(path)
+    raise NotImplementedError
+  end
+
   protected
 
   def batch

app/services/git/repository.rb

@@ -6,8 +6,9 @@ class Git::Repository
   end
 
   def add_git_file(git_file)
+    puts "Adding #{git_file.path}"
     if git_files.empty?
-      action = git_file.should_destroy? ? "Destroy" : "Save"
+      action = should_destroy_file?(git_file) ? "Destroy" : "Save"
       @commit_message = "[#{ git_file.about.class.name }] #{ action } #{ git_file.about }"
     end
     git_files << git_file
@@ -15,10 +16,12 @@ class Git::Repository
 
   def sync!
     return if git_files.empty?
+    puts "Start sync"
     sync_git_files
     mark_as_synced if provider.push(commit_message)
   end
 
+  # Based on content, with the provider's algorithm (sha1 or sha256)
   def computed_sha(string)
     provider.computed_sha(string)
   end
@@ -49,20 +52,72 @@ class Git::Repository
 
   def sync_git_files
     git_files.each do |file|
-      if file.should_create?
+      if should_create_file?(file)
+        puts "Syncing - Creating #{file.path}"
         provider.create_file file.path, file.to_s
-      elsif file.should_update?
+      elsif should_update_file?(file)
+        puts "Syncing - Updating #{file.path}"
         provider.update_file file.path, file.previous_path, file.to_s
-      elsif file.should_destroy?
+      elsif should_destroy_file?(file)
+        puts "Syncing - Destroying #{file.previous_path}"
         provider.destroy_file file.previous_path
+      else
+        puts "Syncing - Nothing to do with #{file.path}"
       end
     end
   end
 
+  # TODO Arnaud : Nettoyer / Rendre + élégant
+
+  def should_create_file?(file)
+    !should_destroy_file?(file) &&
+    !file_exists_on_git?(file) &&
+    (
+      !file_synchronized_with_git?(file) ||
+      file.previous_path.nil? ||
+      file.previous_sha.nil?
+    )
+  end
+
+  def should_update_file?(file)
+    !should_destroy_file?(file) &&
+    (
+      file.previous_path != file.path ||
+      file.previous_sha != computed_sha(file.to_s)
+    )
+  end
+
+  def should_destroy_file?(file)
+    file.will_be_destroyed ||
+    file.path.nil?
+  end
+
+  def file_exists_on_git?(file)
+    git_sha(file.previous_path).present? || # The file exists where it was last time
+    (
+      file.previous_path.nil? &&  # Never saved in the database
+      git_sha(file.path).present? # but it exists in the git repo
+    )
+  end
+
+  def file_synchronized_with_git?(file)
+    file_exists_on_git?(file) && # File exists
+    file.previous_path == file.path && # at the same place
+    git_sha(file.path) == file.previous_sha # with the same content
+  end
+
   def mark_as_synced
+    puts "Marking as synced"
     git_files.each do |git_file|
-      path = git_file.path
-      sha = provider.git_sha path
+      if should_destroy_file?(git_file)
+        path = nil
+        sha = nil
+      else
+        path = git_file.path
+        # TODO Arnaud : Invalider le cache des tree et hash_for_paths pour GitHub pour faire un appel API au lieu de N calculs de SHA
+        sha = computed_sha(git_file.to_s)
+      end
+      puts "Marking #{path}"
       git_file.update previous_path: path,
                       previous_sha: sha
     end