Determining if a file or URL is a git repo or archive

Question

I'm getting these three offenses:

• libraries/helpers.rb:4:1: C: Method has too many lines. [11/10] def get_files(url, file, destination) ^^^

• libraries/helpers.rb:18:1: C: Assignment Branch Condition size for unpack_archive is too high. [16.52/15] def unpack_archive(url, file, destination) ^^^

• libraries/helpers.rb:18:1: C: Method has too many lines. [16/10] def unpack_archive(url, file, destination)

These are all length based offenses. Is there any way to refactor to meet the style guidelines?

# Takes a file and or a url decides if it is a git repo or an archive.
# Downloads or clones it. to the destination.
# Do not enter url with trailing slash.
def get_files(url, file, destination)
  case File.extname(file)
  when '.tar.gz', '.zip' then unpack_archive(url, file, destination)
  when '.git'
    git 'destination' do
      repository "#{url}/#{file}"
      reference 'master'
      user node['apache']['user']
      action :sync
    end
  else fail ArgumentError, "dunno how to handle #{file}"
  end
end

def unpack_archive(url, file, destination)
  remote_file "#{Chef::Config['file_cache_path'] || '/tmp'}/#{file}" do
    owner 'root'
    group 'root'
    mode '0644'
    source "#{url}/#{file}"
    not_if File.readable?(file)
  end
  extract = case File.extname(file)
            when '.tar.gz' then "tar-xC #{destination} -f #{file};"
            when 'zip' then "unzip -d #{destination} -qo #{file};"
            end
  bash "Extract #{file}" do
    cwd ::File.dirname("#{Chef::Config['file_cache_path'] || '/tmp'}/#{file}")
    code   extract
    not_if { ::File.readable(file) }
  end
end

Answer 1

Well the easiest option is to extract parts into separate methods, no?

In unpack_archive the handling of File.extname seems buggy for zip files? At least it's different to the same case statement in get_files.

File.readable? is used, but the other case in bash is missing the question mark.

I don't know enough about Ruby and Chef, but I also saw that the not_if arguments are specified in two different ways; maybe it's possible to use the same way for both directives?

With two additional functions it looks a bit less redundant and more to the point, however I guess there's still space for further improvements:

# Takes a file and or a url decides if it is a git repo or an archive.
# Downloads or clones it. to the destination.
# Do not enter url with trailing slash.
def get_files(url, file, destination)
  case File.extname(file)
  when '.tar.gz', '.zip' then unpack_archive(url, file, destination)
  when '.git' then git_clone(url, file)
  else fail ArgumentError, "dunno how to handle #{file}"
  end
end

def git_clone(url, file)
  git 'destination' do
    repository "#{url}/#{file}"
    reference 'master'
    user node['apache']['user']
    action :sync
  end
end

def get_extract_command(file)
  case File.extname(file)
  when '.tar.gz' then "tar -xC #{destination} -f #{file};"
  when '.zip' then "unzip -d #{destination} -qo #{file};"
  else fail ArgumentError, "dunno how to extract #{file}"
  end
end

def unpack_archive(url, file, destination)
  directory = "#{Chef::Config['file_cache_path'] || '/tmp'}/#{file}"
  remote_file directory do
    owner 'root'
    group 'root'
    mode '0644'
    source "#{url}/#{file}"
    not_if File.readable?(file)
  end
  extract = get_extract_command(file)
  bash "Extract #{file}" do
    cwd ::File.dirname(directory)
    code extract
    not_if { ::File.readable(file) }
  end
end