refactored code based on feedback plus fixed a couple of other issues

db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb

@@ -16,14 +16,12 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration
     say("Projects and Github projects with a wrong URL. It also migrates Gitlab project credentials.")
     in_transaction { process_projects_with_wrong_url }
 
-    say("Migrating bitbucket credentials...")
-    in_transaction { process_project(import_type: 'bitbucket') }
+    say("Migrating bitbucket credentials...")# TODO remove last param
+    in_transaction { process_project(import_type: 'bitbucket', unencrypted_data: ['repo', 'user_map']) }
 
     say("Migrating fogbugz credentials...")
-    in_transaction { process_project(import_type: 'fogbugz') }
+    in_transaction { process_project(import_type: 'fogbugz', unencrypted_data: ['repo', 'user_map']) }
 
-    say("Migrating google code credentials...")
-    in_transaction { process_project(import_type: 'google_code') }
   end
 
   def process_projects_with_wrong_url
@@ -35,19 +33,29 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration
     end
   end
 
-  def process_project(import_type: )
+  def process_project(import_type: , unencrypted_data: [])
     unencrypted_import_data(import_type: import_type).each do |data|
-      replace_data_credentials(data)
+      replace_data_credentials(data, unencrypted_data)
     end
   end
 
-  def replace_data_credentials(data)
-    data_hash = YAML::load(data['data']) if data['data']
+  def replace_data_credentials(data, unencrypted_data)
+    data_hash = JSON.load(data['data']) if data['data']
     if defined?(data_hash) && !data_hash.blank?
-      update_with_encrypted_data(data_hash, data['id'])
+      unencrypted_data_hash = encrypted_data_hash(data_hash, unencrypted_data)
+      update_with_encrypted_data(data_hash, data['id'], unencrypted_data_hash)
     end
   end
 
+  def encrypted_data_hash(data_hash, unencrypted_data)
+    return 'NULL' if unencrypted_data.empty?
+    new_data_hash = {}
+    unencrypted_data.each do |key|
+      new_data_hash[key] = data_hash.delete(key) if data_hash[key]
+    end
+    quote(new_data_hash.to_json)
+  end
+
   def in_transaction
     say_with_time("Processing new transaction...") do
       ActiveRecord::Base.transaction do
@@ -59,18 +67,18 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration
   def update_import_data(import_url, project)
     fake_import_data = FakeProjectImportData.new
     fake_import_data.credentials = import_url.credentials
-    project_import_data = project_import_data(project['id'])
-    if project_import_data
-      execute(update_import_data_sql(project_import_data['id'], fake_import_data))
+    import_data_id = project['import_data_id']
+    if import_data_id
+      execute(update_import_data_sql(import_data_id, fake_import_data))
     else
       execute(insert_import_data_sql(project['id'], fake_import_data))
     end
   end
 
-  def update_with_encrypted_data(data_hash, import_data_id)
+  def update_with_encrypted_data(data_hash, import_data_id, data_array = nil)
     fake_import_data = FakeProjectImportData.new
     fake_import_data.credentials = data_hash
-    execute(update_import_data_sql(import_data_id, fake_import_data))
+    execute(update_import_data_sql(import_data_id, fake_import_data, data_array))
   end
 
   def update_import_url(import_url, project)
@@ -78,16 +86,34 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration
   end
 
   def insert_import_data_sql(project_id, fake_import_data)
-    %( INSERT into project_import_data (encrypted_credentials, project_id, encrypted_credentials_iv, encrypted_credentials_salt) VALUES ( #{quote(fake_import_data.encrypted_credentials)}, '#{project_id}', #{quote(fake_import_data.encrypted_credentials_iv)}, #{quote(fake_import_data.encrypted_credentials_salt)}))
+    %(
+      INSERT INTO project_import_data
+                  (encrypted_credentials,
+                   project_id,
+                   encrypted_credentials_iv,
+                   encrypted_credentials_salt)
+      VALUES      ( #{quote(fake_import_data.encrypted_credentials)},
+                    '#{project_id}',
+                    #{quote(fake_import_data.encrypted_credentials_iv)},
+                    #{quote(fake_import_data.encrypted_credentials_salt)})
+    ).squish
   end
 
   def update_import_data_sql(id, fake_import_data, data = 'NULL')
-    %( UPDATE project_import_data SET encrypted_credentials = #{quote(fake_import_data.encrypted_credentials)}, encrypted_credentials_iv = #{quote(fake_import_data.encrypted_credentials_iv)}, encrypted_credentials_salt = #{quote(fake_import_data.encrypted_credentials_salt)}, data = #{data} WHERE id = '#{id}')
+    %(
+      UPDATE project_import_data
+      SET    encrypted_credentials = #{quote(fake_import_data.encrypted_credentials)},
+             encrypted_credentials_iv = #{quote(fake_import_data.encrypted_credentials_iv)},
+             encrypted_credentials_salt = #{quote(fake_import_data.encrypted_credentials_salt)},
+             data = #{data}
+      WHERE  id = '#{id}'
+    ).squish
   end
 
   #Github projects with token, and any user:password@ based URL
+  #TODO: may need to add import_type != list
   def projects_with_wrong_import_url
-    select_all("SELECT p.id, p.import_url FROM projects p WHERE p.import_url IS NOT NULL AND p.import_url LIKE '%//%@%'")
+    select_all("SELECT p.id, p.import_url, i.id as import_data_id FROM projects p LEFT JOIN project_import_data i on p.id = i.id WHERE p.import_url IS NOT NULL AND p.import_url LIKE '%//%@%'")
   end
 
   # All imports with data for import_type
@@ -95,10 +121,6 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration
     select_all("SELECT i.id, p.import_url, i.data FROM projects p INNER JOIN project_import_data i ON p.id = i.project_id WHERE p.import_url IS NOT NULL AND p.import_type = '#{import_type}' ")
   end
 
-  def project_import_data(project_id)
-    select_one("SELECT id FROM project_import_data WHERE project_id = '#{project_id}'")
-  end
-
   def quote(value)
     ActiveRecord::Base.connection.quote(value)
   end

lib/gitlab/bitbucket_import/client.rb

@@ -5,6 +5,17 @@ module Gitlab
 
       attr_reader :consumer, :api
 
+      def self.from_project(project)
+        credentials = project.import_data.credentials if project.import_data
+        if defined?(credentials) && credentials['bb_session']
+          token = credentials['bb_session']['bitbucket_access_token']
+          token_secret = credentials['bb_session']['bitbucket_access_token_secret']
+          new(token, token_secret)
+        else
+          raise Projects::ImportService::Error, "Unable to find project import data credentials for project ID: #{@project.id}"
+        end
+      end
+
       def initialize(access_token = nil, access_token_secret = nil)
         @consumer = ::OAuth::Consumer.new(
           config.app_id,

lib/gitlab/bitbucket_import/importer.rb

 module Gitlab
   module BitbucketImport
-    class Importer < ImporterInit
+    class Importer
+      attr_reader :project, :client
+
+      def initialize(project)
+        @project = project
+        @client = Client.from_project(@project)
+        @formatter = Gitlab::ImportFormatter.new
+      end
 
       def execute
         import_issues if has_issues?

lib/gitlab/bitbucket_import/importer_init.rb

-module Gitlab
-  module BitbucketImport
-    class ImporterInit
-      attr_reader :project, :client
-
-      def initialize(project)
-        @project = project
-        if import_data_credentials && import_data_credentials['bb_session']
-          token = import_data_credentials['bb_session']['bitbucket_access_token']
-          token_secret = import_data_credentials['bb_session']['bitbucket_access_token_secret']
-          @client = Client.new(token, token_secret)
-          @formatter = Gitlab::ImportFormatter.new
-        else
-          raise Projects::ImportService::Error, "Unable to find project import data credentials for project ID: #{@project.id}"
-        end
-      end
-
-      private
-
-      def import_data_credentials
-        @import_data_credentials ||= project.import_data.credentials if project.import_data
-      end
-    end
-  end
-end

lib/gitlab/bitbucket_import/key_deleter.rb

 module Gitlab
   module BitbucketImport
-    class KeyDeleter  < ImporterInit
-      attr_reader :current_user
+    class KeyDeleter
+      attr_reader :project, :current_user, :client
+
+      def initialize(project)
+        @project = project
+        @current_user = project.creator
+        @client = Client.from_project(@project)
+      end
 
       def execute
         return false unless BitbucketImport.public_key.present?

lib/gitlab/google_code_import/importer.rb

@@ -6,13 +6,12 @@ module Gitlab
       def initialize(project)
         @project = project
 
-        if import_data_credentials && import_data_credentials['repo']
-          @repo = GoogleCodeImport::Repository.new(import_data_credentials['repo'])
-          @closed_statuses = []
-          @known_labels = Set.new
-        else
-          raise Projects::ImportService::Error, "Unable to find project import data credentials for project ID: #{@project.id}"
-        end
+        import_data = project.import_data.try(:data)
+        repo_data = import_data["repo"] if import_data
+        @repo = GoogleCodeImport::Repository.new(repo_data)
+
+        @closed_statuses = []
+        @known_labels = Set.new
       end
 
       def execute
@@ -29,10 +28,6 @@ module Gitlab
 
       private
 
-      def import_data_credentials
-        @import_data_credentials ||= project.import_data.credentials if project.import_data
-      end
-
       def user_map
         @user_map ||= begin
           user_map = Hash.new do |hash, user|
@@ -40,7 +35,8 @@ module Gitlab
             Client.mask_email(user).sub("...", "\\.\\.\\.")
           end
 
-          stored_user_map = import_data_credentials["user_map"]
+          import_data = project.import_data.try(:data)
+          stored_user_map = import_data["user_map"] if import_data
           user_map.update(stored_user_map) if stored_user_map
 
           user_map

lib/gitlab/google_code_import/project_creator.rb

@@ -25,7 +25,7 @@ module Gitlab
         ).execute
 
         project.create_import_data(
-          credentials: {
+          data: {
             "repo"      => repo.raw_data,
             "user_map"  => user_map
           }

spec/lib/gitlab/google_code_import/importer_spec.rb

@@ -15,7 +15,7 @@ describe Gitlab::GoogleCodeImport::Importer, lib: true do
   subject { described_class.new(project) }
 
   before do
-    project.create_import_data(credentials: import_data)
+    project.create_import_data(data: import_data)
   end
 
   describe "#execute" do