Draft: Resolve "Provide exclusive lock information to the frontend in CE as well as EE"

What does this MR do and why?

Work on plugging the gap by the fact LFS can be used in CE but the code doesn't seem to expect file locking to occur in CE because the feature of Path Locks is EE only.

Proposal

In Gitlab::TreeSummary we have a summarize method which is then overridden in EE::Gitlab::TreeSummary#summarize

This override fills the path lock information

      def summarize
        super.tap { |summary| fill_path_locks!(summary) }
      end

      private

      def fill_path_locks!(entries)
        return unless project.feature_available?(:file_locks)

        finder = ::Gitlab::PathLocksFinder.new(project)
        paths = entries.map { |entry| entry_path(entry) }
        finder.preload_for_paths(paths)

        entries.each do |entry|
          path = entry_path(entry)
          path_lock = finder.find_by_path(path)

          entry[:lock_label] = path_lock && text_label_for_lock(path_lock, path)
        end
      end

      def entry_path(entry)
        File.join(*[path, entry[:file_name]].compact).force_encoding(Encoding::ASCII_8BIT)
      end

Important note

If project.lfs_enabled? and licensed_feature_available?(:file_locks) in EE we sync the 2 features when an LFS file is locked so any LfsFileLock will also have an accompanying PathLock if file locks are enabled. However, in file locks is an EE feature so in CE we will always only be able to find LfsFileLocks, but only if project.lfs_enabled?.

Due to this quirk I think we can add logic to CE to check for LfsFileLocks if project.lfs_enabled? and then keep the fill_path_locks! override in EE but instead of

      def fill_path_locks!(entries)
        return unless project.feature_available?(:file_locks)

it would return super.

      def fill_path_locks!(entries)
        return super unless project.feature_available?(:file_locks)

We can remove the override for summarize and move the logic for this into CE

-      def summarize
-        super.tap { |summary| fill_path_locks!(summary) }
-      end
    def summarize
      return [] if offset < 0

      commits_hsh = fetch_last_cached_commits_list
      prerender_commit_full_titles!(commits_hsh.values)

      entries = commits_hsh.map do |path_key, commit|
        commit = cache_commit(commit)

        {
          file_name: File.basename(path_key).force_encoding(Encoding::UTF_8),
          commit: commit,
          commit_path: commit_path(commit),
          commit_title_html: markdown_field(commit, :full_title)
        }
      end
      fill_path_locks!(entries)
    end

    private

    def fill_path_locks!(entries)
      return unless project.lfs_enabled?

      find the lfs file locks and do the same thing as EE
    end

Refactoring the solution to find the locks before looping?

I'm not sure if this would improve anything but it may be possible to remove a loop here if initialize the finder before mapping the commits_hsh.map do |path_key, commit| so we lazy load the locks as needed instead of looping once, then taking all of the paths and looping again.

References

Screenshots or screen recordings

Before After

How to set up and validate locally

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #534073

Edited by Pranav Jain

Merge request reports

Loading