From 940613ae347af450426887a72873cdc41df445fb Mon Sep 17 00:00:00 2001
From: Artur Beljajev <artur.beljajev@internet.ee>
Date: Wed, 7 Feb 2018 02:17:48 +0200
Subject: [PATCH] Decompose EppSession#data

#700
---
 .reek                                      |  2 --
 .rubocop_todo.yml                          |  5 ---
 app/controllers/epp/sessions_controller.rb |  2 +-
 app/controllers/epp_controller.rb          |  2 +-
 app/models/epp_session.rb                  | 36 ++--------------------
 lib/tasks/dev.rake                         |  2 +-
 test/fixtures/epp_sessions.yml             |  4 +--
 test/models/epp_session_test.rb            | 17 ++++------
 8 files changed, 13 insertions(+), 57 deletions(-)

diff --git a/.reek b/.reek
index 3bdfebb81..b621c44a5 100644
--- a/.reek
+++ b/.reek
@@ -538,7 +538,6 @@ IrresponsibleModule:
   - DomainStatus
   - DomainTransfer
   - Epp::Contact
-  - EppSession
   - Invoice
   - InvoiceItem
   - Keyrelay
@@ -1027,7 +1026,6 @@ PrimaDonnaMethod:
   - Contact
   - Domain
   - Epp::Domain
-  - EppSession
   - RegistrantVerification
   - Registrar
 BooleanParameter:
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index a9c491df9..7acf2bd1b 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -655,11 +655,6 @@ Performance/StringReplacement:
     - 'app/models/directo.rb'
     - 'app/models/dnskey.rb'
 
-# Offense count: 1
-Security/MarshalLoad:
-  Exclude:
-    - 'app/models/epp_session.rb'
-
 # Offense count: 4
 # Cop supports --auto-correct.
 # Configuration parameters: EnforcedStyle, SupportedStyles.
diff --git a/app/controllers/epp/sessions_controller.rb b/app/controllers/epp/sessions_controller.rb
index dc67405d1..8f4d14ac7 100644
--- a/app/controllers/epp/sessions_controller.rb
+++ b/app/controllers/epp/sessions_controller.rb
@@ -91,7 +91,7 @@ class Epp::SessionsController < EppController
         end
       end
 
-      epp_session[:api_user_id] = @api_user.id
+      epp_session.user = @api_user
       epp_session.update_column(:registrar_id, @api_user.registrar_id)
       render_epp_response('login_success')
     else
diff --git a/app/controllers/epp_controller.rb b/app/controllers/epp_controller.rb
index 254cb8968..07d3286d0 100644
--- a/app/controllers/epp_controller.rb
+++ b/app/controllers/epp_controller.rb
@@ -115,7 +115,7 @@ class EppController < ApplicationController
   end
 
   def current_user
-    @current_user ||= ApiUser.find_by_id(epp_session[:api_user_id])
+    @current_user ||= epp_session.user
     # by default PaperTrail uses before filter and at that
     # time current_user is not yet present
     ::PaperTrail.whodunnit = user_log_str(@current_user)
diff --git a/app/models/epp_session.rb b/app/models/epp_session.rb
index 8d36e15ef..dd97a91a9 100644
--- a/app/models/epp_session.rb
+++ b/app/models/epp_session.rb
@@ -1,38 +1,6 @@
 class EppSession < ActiveRecord::Base
-  before_save :marshal_data!
+  belongs_to :user, required: true
+  belongs_to :registrar
 
   validates :session_id, presence: true
-
-  belongs_to :registrar
-  # rubocop: disable Rails/ReadWriteAttribute
-  # Turned back to read_attribute, thus in Rails 4
-  # there is differences between self[:data] and read_attribute.
-  def data
-    @data ||= self.class.unmarshal(read_attribute(:data)) || {}
-  end
-  # rubocop: enable Rails/ReadWriteAttribute
-
-  def [](key)
-    data[key.to_sym]
-  end
-
-  def []=(key, value)
-    data[key.to_sym] = value
-    save!
-  end
-
-  def marshal_data!
-    self.data = self.class.marshal(data)
-  end
-
-  class << self
-    def marshal(data)
-      ::Base64.encode64(Marshal.dump(data)) if data
-    end
-
-    def unmarshal(data)
-      return data unless data.is_a? String
-      Marshal.load(::Base64.decode64(data)) if data
-    end
-  end
 end
diff --git a/lib/tasks/dev.rake b/lib/tasks/dev.rake
index e25053133..d0c855903 100644
--- a/lib/tasks/dev.rake
+++ b/lib/tasks/dev.rake
@@ -52,7 +52,7 @@ namespace :dev do
       epp_session = EppSession.new
       epp_session.session_id = 'test'
       epp_session.registrar = registrar
-      epp_session[:api_user_id] = api_user.id
+      epp_session.user = api_user
       epp_session.save!
 
       domain_counter = 1.step
diff --git a/test/fixtures/epp_sessions.yml b/test/fixtures/epp_sessions.yml
index 8cdaedb8c..9d824cf1c 100644
--- a/test/fixtures/epp_sessions.yml
+++ b/test/fixtures/epp_sessions.yml
@@ -1,9 +1,9 @@
 api_bestnames:
   session_id: 1
+  user: api_bestnames
   registrar: bestnames
-  data: <%= Base64.encode64(Marshal.dump({api_user_id: ActiveRecord::Fixtures.identify(:api_bestnames)})) %>
 
 api_goodnames:
   session_id: 2
+  user: api_goodnames
   registrar: goodnames
-  data: <%= Base64.encode64(Marshal.dump({api_user_id: ActiveRecord::Fixtures.identify(:api_goodnames)})) %>
diff --git a/test/models/epp_session_test.rb b/test/models/epp_session_test.rb
index b4801bb5a..98ae993f5 100644
--- a/test/models/epp_session_test.rb
+++ b/test/models/epp_session_test.rb
@@ -9,20 +9,15 @@ class EppSessionTest < ActiveSupport::TestCase
     assert @epp_session.valid?
   end
 
-  def test_api_user_id_serialization
-    epp_session = EppSession.new
-    epp_session.session_id = 'test'
-    epp_session.registrar = registrars(:bestnames)
-    epp_session[:api_user_id] = ActiveRecord::Fixtures.identify(:api_bestnames)
-    epp_session.save!
-    epp_session.reload
-
-    assert_equal ActiveRecord::Fixtures.identify(:api_bestnames), epp_session[:api_user_id]
-  end
-
   def test_session_id_presence_validation
     @epp_session.session_id = nil
     @epp_session.validate
     assert @epp_session.invalid?
   end
+
+  def test_user_presence_validation
+    @epp_session.user = nil
+    @epp_session.validate
+    assert @epp_session.invalid?
+  end
 end