1
\$\begingroup\$

I created a script to automate the task of uploading a CSV file to Box. I have a couple of files a Box client and a MySQL client. I create an instance of both on the function.rb file which is the one below. I wonder if there is anything to improve? if there are principles I'm not following or if I'm breaking best practices.

Is there anything that can be improved?

require 'json'
require 'date'
require 'dotenv/load'
require 'fileutils'
require 'csv'
require 'logger'
require './my_sql'
require './box_api'
require 'pry'

begin
  year = ARGV[0]
  month = ARGV[1]
  day = ARGV[2]

  search_timestamp = Time.new(year, month, day).utc

  db_client = MySQL.new(search_timestamp)

  emails = db_client.get_emails_from_db
  return 'No new emails found' if emails.entries.empty?

  box = BoxApi.new(ENV['BOX_USER_ID'])

  date = DateTime.now.strftime("%m-%d-%Y").to_s
  file_name = "access-emails-#{date}"

  CSV.open("./tmp/#{file_name}.csv", "wb") do |csv|
    emails.entries.each do |entrie|
      csv << [entrie.values[0], entrie.values[1]]
    end
  end

  box.upload_file_to_box("./tmp/#{file_name}.csv", file_name, ENV['BOX_FOLDER_ID'])

  FileUtils.rm("./tmp/#{file_name}.csv")

  puts "successfully uploaded CSV file to Box"
rescue StandardError => e
  logger = Logger.new(STDOUT)
  logger.level = ENV.fetch('LOG_LEVEL', Logger::INFO)
  logger.datetime_format = '%Y-%m-%d %H:%M:%S '
  logger.error("msg: #{e}, trace: #{e.backtrace.join("\n")}")
end
\$\endgroup\$
1
\$\begingroup\$

I think your script would benefit from some object oriented composition. Basically you have three different concerns.

  • Config
  • File backup (select from database and storage in CSV)
  • File upload

A few things could change now, for instance backup from a different source (different database, cloud etc), upload to a different remote service (e.g. dropbox). Additionally, having small composable objects would make it easier to test this.

Here are some examples how to compose your script more object oriented.

Config

We could use an OpenStruct to store our config data. This way we only need to write our environment variables once, if we want to change them later there is only one place to update them.

require 'ostruct'

config = OpenStruct.new(
  year: ARGV[0],
  month: ARGV[1],
  day: ARGV[2],
  box_user_id: ENV['BOX_USER_ID'],
  box_folder_id: ENV['BOX_FOLDER_ID']
)

FileBackup

We can extract a backup file which just excepts rows and writes them to a CSV file. The dependency injection makes it also easier to test this (e.g. inject the data to write and the test directory)

class BackupFile
  def initialize(rows:, date: DateTime.now.strftime("%m-%d-%Y").to_s, directory: "./tmp")
    @rows = rows
    @date = date
  end

  def save
    CSV.open(full_path, "wb") do |csv|
      rows.each do |entry|
        csv << [entry.values[0], entry.values[1]]
      end
    end
  end

  def full_path
    File.join(directory, filename)
  end

  def delete
    FileUtils.rm(full_path)
  end

  private

  attr_reader :rows, :date

  def file_name
    "access-emails-#{date}"
  end
end


db_client = MySQL.new(search_timestamp)
emails = db_client.get_emails_from_db
return 'No new emails found' if emails.entries.empty?

file = BackupFile.new(emails.entries)
file.save

Upload

The uploader accepts a client, path and remote folder. Also notice that we have an adapter around the BoxApi to implement a common interface upload. If we want to swap it out to upload to Dropbox, we only need to write a DropboxClient adapter which we can inject into the uploader. To test, we can write even a TestClient.

class Uploader
  def initialize(client:, path:, remote_folder:)
    @client = client
    @path = path
    @remote_folder = remote_folder
  end

  def upload
    client.upload(path, file_name, remote_folder)
  end

  private

  attr_reader :client, :path, :remote_folder

  def file_name
    File.basename(path)
  end
end

class BoxClient
  def initialize(client:, box_user_id:)
    @client = client.new(box_user_id)
  end

  def upload(path, file_name, remote_folder)
    client.upload_file_to_box(path, file_name, remote_folder)
  end

  private

  attr_reader :client
end

Error handling

I would move the error handling into the classes directly and also inject the logger. Something like this:

class BoxClient
  def initialize(client:, box_user_id:, logger: Logger.new)
    @client = client.new(box_user_id)
  end

  def upload(path, file_name, remote_folder)
    client.upload_file_to_box(path, file_name, remote_folder)
  rescue BoxError =>
    logger.error("Upload failed: #{e.message}")
  end

  private

  attr_reader :client
end
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.