Refactored model and removed rubocop findings

app/controllers/urlinfos_controller.rb

@@ -1,8 +1,6 @@
 # The controller to provide urlinfo service
 class UrlinfosController < ApplicationController
-  before_action :set_urlinfo, only: [:show, :update, :destroy]
-  before_action :fetch_urlinfo_from_storage, only: [:find_by_url]
-  before_action :fetch_urlinfo_from_database, only: [:delete_by_url]
+  before_action :set_urlinfo, only: %i[show update destroy]
 
   def index
     @urlinfos = Urlinfo.all
@@ -24,24 +22,17 @@ def destroy
   end
 
   def find_by_url
-    json_response(@result)
+    lookup_result = Urlinfo.new.find_urlinfo_in_storage(params)
+    json_response(lookup_result)
   end
 
   def create_by_url
-    # 1. generating full uri from domain_name and query_string
-    #     cache stores full uri as key and malware flag as value
-    param_url = generate_url_from_params 
-    # 2. making empty response object with full uri
-    Urlinfo.make_empty_response_object(param_url)
-    fetch_urlinfo_from_database(params)
-    store_urlinfo_to_database(@urlinfo, params)
-    @result[:data_from] = "database"
-    @result[:malware] = true 
-    json_response(@result, :created)
+    lookup_result = Urlinfo.new.create_urlinfo_in_database(params)
+    json_response(lookup_result, :created)
   end
 
   def delete_by_url
-    @urlinfo.destroy_all
+    Urlinfo.new.destroy_urlinfo_from_database(params)
     head :no_content
   end
 
@@ -50,71 +41,4 @@ def delete_by_url
   def set_urlinfo
     @urlinfo = Urlinfo.find(params[:id])
   end
-
-  def store_urlinfo_to_database(@urlinfo, params)
-    if @urlinfo.nil?
-      @urlinfo = Urlinfo.create!(url: param_url,
-                                 malware: true,
-                                 created_by: params[:requested_by],
-                                 domain_name: params[:domain_name],
-                                 query_string: params[:query_string])
-    else
-      @urlinfo.update(url: param_url,
-                      malware: true,
-                      created_by: params[:requested_by],
-                      domain_name: params[:domain_name],
-                      query_string: params[:query_string])
-    end
-  end
-
-  def fetch_urlinfo_from_storage
-    # 1. generating full uri from domain_name and query_string
-    #     cache stores full uri as key and malware flag as value
-    param_url = generate_url_from_params
-    # 2. making empty response object with full uri
-    make_empty_response_object(param_url)
-    # 3. looking up the requested uri in cache first
-    cached_value = fetch_urlinfo_from_cache(param_url)
-    if cached_value.nil?
-      # 4. if not found in cache then looking it up in database
-      fetch_urlinfo_from_database(params)
-    end
-  end
-
-  def fetch_urlinfo_from_database(params)
-    @urlinfo = Urlinfo.where(domain_name: params[:domain_name], query_string: params[:query_string]).first
-    malware_flag = @urlinfo.nil? ? false : true
-    @result[:data_from] = "database"
-    @result[:malware] = malware_flag
-    param_url = generate_url_from_params
-    Urlinfo.set_url_in_cache(param_url, malware_flag)
-  end
-
-  def fetch_urlinfo_from_cache(param_url)
-    cached_value = Urlinfo.get_url_in_cache(param_url)
-    if cached_value.nil?
-      return nil
-    else
-      # updating cached url again to prevent the data from being expired (LRU)
-      Urlinfo.set_url_in_cache(param_url, cached_value)
-      @result[:malware] = cached_value == "true" ? true : false
-      @result[:data_from] = "cache"
-    end
-  end
-
-  def make_empty_response_object(param_url)
-    @result = {
-      url: param_url,
-      malware: nil,
-      data_from: nil
-    }
-  end
-
-  def generate_url_from_params
-    if params[:query_string].nil?
-      "#{params[:domain_name]}"
-    else
-      "#{params[:domain_name]}/?#{params[:query_string]}"
-    end
-  end
 end

app/models/urlinfo.rb

@@ -3,31 +3,101 @@ class Urlinfo < ApplicationRecord
   # validations
   validates_presence_of :url, :malware, :created_by, :domain_name
 
-  after_save :set_url_as_malware_in_cache
-  after_destroy :set_url_as_clean_in_cache
+  def get_url_in_cache(url)
+    cache = get_cache_connection_by_url(url)
+    cache.get(url)
+  end
 
-  def set_url_as_malware_in_cache
-    cache = Urlinfo.get_cache_connection_by_url(url)
-    cache[url] = true
+  def set_urlinfo_in_cache(url, malware)
+    cache = get_cache_connection_by_url(url)
+    cache[url] = malware
   end
 
-  def set_url_as_clean_in_cache
-    cache = Urlinfo.get_cache_connection_by_url(url)
-    cache[url] = false
+  def find_urlinfo_in_storage(params)
+    param_url = generate_url_from_params(params)
+    cached_value = get_url_in_cache(param_url)
+    data_source = 'cache'
+    malware = false
+    if cached_value.nil?
+      urlinfo = find_urlinfo(params)
+      unless urlinfo.nil?
+        data_source = 'database'
+        malware = true
+      end
+    else
+      malware = cached_value == 'true' ? true : false
+    end
+    set_urlinfo_in_cache(param_url, malware)
+
+    { url: param_url, data_source: data_source, malware: malware }
   end
 
-  def self.get_url_in_cache(url)
-    cache = Urlinfo.get_cache_connection_by_url(url)
-    cache.get(url)
+  def create_urlinfo_in_database(params)
+    param_url = generate_url_from_params(params)
+    urlinfo = find_urlinfo(params)
+    if urlinfo.nil?
+      create_urlinfo(param_url, params)
+    else
+      update_urlinfo(urlinfo, param_url, params)
+    end
+    set_urlinfo_in_cache(param_url, true)
+
+    { url: param_url, data_source: 'database', malware: true }
   end
 
-  def self.set_url_in_cache(url, malware_flag)
-    cache = Urlinfo.get_cache_connection_by_url(url)
-    cache[url] = malware_flag
+  def destroy_urlinfo_from_database(params)
+    param_url = generate_url_from_params(params)
+    Urlinfo.where(
+      domain_name: params[:domain_name],
+      query_string: params[:query_string]
+    ).destroy_all
+    set_urlinfo_in_cache(param_url, false)
+
+    { url: param_url, data_source: 'cache', malware: false }
   end
 
   private
 
+  def find_urlinfo(params)
+    Urlinfo.where(
+      domain_name:  params[:domain_name],
+      query_string: params[:query_string]
+    ).first
+  end
+
+  def create_urlinfo(param_url, params)
+    Urlinfo.create!(
+      url:          param_url,
+      malware:      true,
+      created_by:   params[:requested_by],
+      domain_name:  params[:domain_name],
+      query_string: params[:query_string]
+    )
+  end
+
+  def update_urlinfo(urlinfo, param_url, params)
+    urlinfo.update(
+      url: param_url,
+      malware: true,
+      created_by: params[:requested_by],
+      domain_name: params[:domain_name],
+      query_string: params[:query_string]
+    )
+  end
+
+  def set_url_in_cache(url, malware_flag)
+    cache = get_cache_connection_by_url(url)
+    cache[url] = malware_flag
+  end
+
+  def generate_url_from_params(params)
+    if params[:query_string].nil?
+      params[:domain_name]
+    else
+      "#{params[:domain_name]}/?#{params[:query_string]}"
+    end
+  end
+
   def get_cache_connection_by_url(url)
     hashcode = (url.length % UrlLookupCache.redis.length)
     UrlLookupCache.redis[hashcode][:object]

spec/controllers/urls_controller_spec.rb

@@ -1,5 +1,4 @@
 require 'rails_helper'
 
 RSpec.describe UrlinfosController, type: :controller do
-
 end

spec/factories/urlinfo.rb

@@ -1,9 +1,9 @@
 FactoryGirl.define do
   factory :urlinfo do
-    domain_name { "#{Faker::Internet.domain_name}" }
-    query_string { "q=abc" }
+    domain_name { Faker::Internet.domain_name.to_s }
+    query_string { 'q=abc' }
     url { "#{domain_name}/?#{query_string}" }
     malware { true }
-    created_by { "1" }
+    created_by { '1' }
   end
 end

spec/requests/urlinfos_spec.rb

@@ -46,7 +46,7 @@
       end
 
       it 'returns a not found message' do
-        expect(response.body).to match (/Couldn't find Url/)
+        expect(response.body).to match(/Couldn't find Url/)
       end
     end
   end
@@ -73,7 +73,6 @@
 
       it 'creates a urlinfo' do
         expect(json['url']).to eq('www.google.com/?q=abc')
-        expect(json['domain_name']).to eq('www.google.com')
         expect(json['malware']).to eq(true)
       end
 
@@ -123,7 +122,7 @@
   end
 
   describe 'DELETE /urlinfo/:created_by/:domain_name/:query_string' do
-    before { delete "/urlinfo/1/www.google.com/q=abc" }
+    before { delete '/urlinfo/1/www.google.com/q=abc' }
 
     it 'returns status code 204' do
       expect(response).to have_http_status(204)