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