From c8997749391aa879f209097c73cc50373b6dc300 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Gaya?= <sebastien.gaya@gmail.com> Date: Fri, 29 Jul 2022 11:25:03 +0200 Subject: [PATCH] refactor git --- app/models/communication/website/git_file.rb | 50 --------------- app/services/git/providers/abstract.rb | 24 +++++++ app/services/git/repository.rb | 67 ++++++++++++++++++-- 3 files changed, 85 insertions(+), 56 deletions(-) diff --git a/app/models/communication/website/git_file.rb b/app/models/communication/website/git_file.rb index 764e32104..87795be36 100644 --- a/app/models/communication/website/git_file.rb +++ b/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 diff --git a/app/services/git/providers/abstract.rb b/app/services/git/providers/abstract.rb index bd3778d0f..be21a999e 100644 --- a/app/services/git/providers/abstract.rb +++ b/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 diff --git a/app/services/git/repository.rb b/app/services/git/repository.rb index 318604565..67c3c39c9 100644 --- a/app/services/git/repository.rb +++ b/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 -- GitLab