From e6e03a1a9612a57e78b6b4e4137cfe3625d0111f Mon Sep 17 00:00:00 2001
From: Benjamin Kampmann <ben.kampmann@gmail.com>
Date: Fri, 25 Apr 2014 14:10:54 +0200
Subject: [PATCH 1/8] move custom fields into its own concern

---
 app/models/user.rb                            | 31 +-------
 lib/concern/has_custom_fields.rb              | 39 +++++++++
 .../concern/has_custom_fields_spec.rb         | 79 +++++++++++++++++++
 3 files changed, 120 insertions(+), 29 deletions(-)
 create mode 100644 lib/concern/has_custom_fields.rb
 create mode 100644 spec/components/concern/has_custom_fields_spec.rb

diff --git a/app/models/user.rb b/app/models/user.rb
index 5ee6a10c586..a7f02c5af86 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -8,10 +8,12 @@ require_dependency 'post_destroyer'
 require_dependency 'user_name_suggester'
 require_dependency 'pretty_text'
 require_dependency 'url_helper'
+require_dependency 'concern/has_custom_fields'
 
 class User < ActiveRecord::Base
   include Roleable
   include UrlHelper
+  include Concern::HasCustomFields
 
   has_many :posts
   has_many :notifications, dependent: :destroy
@@ -31,7 +33,6 @@ class User < ActiveRecord::Base
   has_many :invites, dependent: :destroy
   has_many :topic_links, dependent: :destroy
   has_many :uploads
-  has_many :user_custom_fields, dependent: :destroy
 
   has_one :facebook_user_info, dependent: :destroy
   has_one :twitter_user_info, dependent: :destroy
@@ -69,7 +70,6 @@ class User < ActiveRecord::Base
 
   after_save :update_tracked_topics
   after_save :clear_global_notice_if_needed
-  after_save :save_custom_fields
 
   after_create :create_email_token
   after_create :create_user_stat
@@ -589,35 +589,8 @@ class User < ActiveRecord::Base
     nil
   end
 
-  def custom_fields
-    @custom_fields ||= begin
-      @custom_fields_orig = Hash[*user_custom_fields.pluck(:name,:value).flatten]
-      @custom_fields_orig.dup
-    end
-  end
-
   protected
 
-  def save_custom_fields
-    if @custom_fields && @custom_fields_orig != @custom_fields
-      dup = @custom_fields.dup
-
-      user_custom_fields.each do |f|
-        if dup[f.name] != f.value
-          f.destroy
-        else
-          dup.remove[f.name]
-        end
-      end
-
-      dup.each do |k,v|
-        user_custom_fields.create(name: k, value: v)
-      end
-
-      @custom_fields_orig = @custom_fields
-    end
-  end
-
   def cook
     if bio_raw.present?
       self.bio_cooked = PrettyText.cook(bio_raw, omit_nofollow: self.has_trust_level?(:leader)) if bio_raw_changed?
diff --git a/lib/concern/has_custom_fields.rb b/lib/concern/has_custom_fields.rb
new file mode 100644
index 00000000000..18273af10e0
--- /dev/null
+++ b/lib/concern/has_custom_fields.rb
@@ -0,0 +1,39 @@
+module Concern
+  module HasCustomFields
+    extend ActiveSupport::Concern
+
+    included do
+      has_many :_custom_fields, dependent: :destroy, :class_name => "#{name}CustomField"
+      after_save :save_custom_fields
+    end
+
+    def custom_fields
+      @custom_fields ||= begin
+        @custom_fields_orig = Hash[*_custom_fields.pluck(:name,:value).flatten]
+        @custom_fields_orig.dup
+      end
+    end
+
+    protected
+
+    def save_custom_fields
+      if @custom_fields && @custom_fields_orig != @custom_fields
+        dup = @custom_fields.dup
+
+        _custom_fields.each do |f|
+          if dup[f.name] != f.value
+            f.destroy
+          else
+            dup.remove[f.name]
+          end
+        end
+
+        dup.each do |k,v|
+          _custom_fields.create(name: k, value: v)
+        end
+
+        @custom_fields_orig = @custom_fields
+      end
+    end
+  end
+end
\ No newline at end of file
diff --git a/spec/components/concern/has_custom_fields_spec.rb b/spec/components/concern/has_custom_fields_spec.rb
new file mode 100644
index 00000000000..176e1c0cc96
--- /dev/null
+++ b/spec/components/concern/has_custom_fields_spec.rb
@@ -0,0 +1,79 @@
+require "spec_helper"
+require_dependency "concern/has_custom_fields"
+
+describe Concern::HasCustomFields do
+
+  context "custom_fields" do
+    before do
+
+      Topic.exec_sql("create temporary table test_items(id SERIAL primary key)")
+      Topic.exec_sql("create temporary table test_item_custom_fields(id SERIAL primary key, test_item_id int, name varchar(256) not null, value text)")
+
+      class TestItem < ActiveRecord::Base
+        include Concern::HasCustomFields
+      end
+
+      class TestItemCustomField < ActiveRecord::Base
+        belongs_to :test_item
+      end
+    end
+
+    after do
+      Topic.exec_sql("drop table test_items")
+      Topic.exec_sql("drop table test_item_custom_fields")
+
+      # import is making my life hard, we need to nuke this out of orbit
+      des = ActiveSupport::DescendantsTracker.class_variable_get :@@direct_descendants
+      des[ActiveRecord::Base].delete(TestItem)
+      des[ActiveRecord::Base].delete(TestItemCustomField)
+    end
+
+    it "simple modification of custom fields" do
+      test_item = TestItem.new
+
+      test_item.custom_fields["a"].should == nil
+
+      test_item.custom_fields["bob"] = "marley"
+      test_item.custom_fields["jack"] = "black"
+      test_item.save
+
+      test_item = TestItem.find(test_item.id)
+
+      test_item.custom_fields["bob"].should == "marley"
+      test_item.custom_fields["jack"].should == "black"
+
+      test_item.custom_fields.delete("bob")
+      test_item.custom_fields["jack"] = "jill"
+
+      test_item.save
+      test_item = TestItem.find(test_item.id)
+
+      test_item.custom_fields.should == {"jack" => "jill"}
+    end
+
+
+    it "simple modifications don't interfere" do
+      test_item = TestItem.new
+
+      test_item.custom_fields["a"].should == nil
+
+      test_item.custom_fields["bob"] = "marley"
+      test_item.custom_fields["jack"] = "black"
+      test_item.save
+
+      test_item2 = TestItem.new
+
+      test_item2.custom_fields["x"].should == nil
+
+      test_item2.custom_fields["sixto"] = "rodriguez"
+      test_item2.custom_fields["de"] = "la playa"
+      test_item2.save
+
+      test_item = TestItem.find(test_item.id)
+      test_item2 = TestItem.find(test_item2.id)
+
+      test_item.custom_fields.should == {"jack" => "black", "bob" => "marley"}
+      test_item2.custom_fields.should == {"sixto" => "rodriguez", "de" => "la playa"}
+    end
+  end
+end

From 2450088c03719db0c17e71dfc44f48a7862b74b3 Mon Sep 17 00:00:00 2001
From: Benjamin Kampmann <ben.kampmann@gmail.com>
Date: Fri, 25 Apr 2014 15:14:05 +0200
Subject: [PATCH 2/8] Add CustomFields to Post, Category and Group

---
 app/models/category.rb                        |  3 ++
 app/models/category_custom_field.rb           | 19 +++++++++++++
 app/models/group.rb                           |  4 +++
 app/models/group_custom_field.rb              | 19 +++++++++++++
 app/models/post.rb                            |  2 ++
 app/models/post_custom_field.rb               | 19 +++++++++++++
 .../20140425125742_add_custom_fields.rb       | 28 +++++++++++++++++++
 spec/models/category_spec.rb                  | 12 ++++++++
 spec/models/group_spec.rb                     | 13 +++++++++
 spec/models/post_spec.rb                      | 12 ++++++++
 10 files changed, 131 insertions(+)
 create mode 100644 app/models/category_custom_field.rb
 create mode 100644 app/models/group_custom_field.rb
 create mode 100644 app/models/post_custom_field.rb
 create mode 100644 db/migrate/20140425125742_add_custom_fields.rb

diff --git a/app/models/category.rb b/app/models/category.rb
index 1b512899987..4bd71f5be4b 100644
--- a/app/models/category.rb
+++ b/app/models/category.rb
@@ -1,6 +1,9 @@
+require_dependency "concern/has_custom_fields"
+
 class Category < ActiveRecord::Base
 
   include Positionable
+  include Concern::HasCustomFields
 
   belongs_to :topic, dependent: :destroy
   belongs_to :topic_only_relative_url,
diff --git a/app/models/category_custom_field.rb b/app/models/category_custom_field.rb
new file mode 100644
index 00000000000..ccc3344ff8b
--- /dev/null
+++ b/app/models/category_custom_field.rb
@@ -0,0 +1,19 @@
+class CategoryCustomField < ActiveRecord::Base
+  belongs_to :category
+end
+
+# == Schema Information
+#
+# Table name: category_custom_fields
+#
+#  id           :integer          not null, primary key
+#  category_id  :integer          not null
+#  name         :string(256)      not null
+#  value        :text
+#  created_at   :datetime
+#  updated_at   :datetime
+#
+# Indexes
+#
+#  index_category_custom_fields_on_category_id_and_name  (category_id,name)
+#
diff --git a/app/models/group.rb b/app/models/group.rb
index 2741b30dfcb..35933da1012 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -1,4 +1,8 @@
+require_dependency "concern/has_custom_fields"
+
 class Group < ActiveRecord::Base
+  include Concern::HasCustomFields
+
   has_many :category_groups
   has_many :group_users, dependent: :destroy
 
diff --git a/app/models/group_custom_field.rb b/app/models/group_custom_field.rb
new file mode 100644
index 00000000000..78b087b97c8
--- /dev/null
+++ b/app/models/group_custom_field.rb
@@ -0,0 +1,19 @@
+class GroupCustomField < ActiveRecord::Base
+  belongs_to :group
+end
+
+# == Schema Information
+#
+# Table name: group_custom_fields
+#
+#  id           :integer          not null, primary key
+#  group_id     :integer          not null
+#  name         :string(256)      not null
+#  value        :text
+#  created_at   :datetime
+#  updated_at   :datetime
+#
+# Indexes
+#
+#  index_group_custom_fields_on_group_id_and_name  (group_id,name)
+#
diff --git a/app/models/post.rb b/app/models/post.rb
index 3d2c766ce97..fc38fc40798 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -6,6 +6,7 @@ require_dependency 'enum'
 require_dependency 'post_analyzer'
 require_dependency 'validators/post_validator'
 require_dependency 'plugin/filter'
+require_dependency "concern/has_custom_fields"
 
 require 'archetype'
 require 'digest/sha1'
@@ -13,6 +14,7 @@ require 'digest/sha1'
 class Post < ActiveRecord::Base
   include RateLimiter::OnCreateRecord
   include Trashable
+  include Concern::HasCustomFields
 
   rate_limit
   rate_limit :limit_posts_per_day
diff --git a/app/models/post_custom_field.rb b/app/models/post_custom_field.rb
new file mode 100644
index 00000000000..e7b00105660
--- /dev/null
+++ b/app/models/post_custom_field.rb
@@ -0,0 +1,19 @@
+class PostCustomField < ActiveRecord::Base
+  belongs_to :post
+end
+
+# == Schema Information
+#
+# Table name: post_custom_fields
+#
+#  id           :integer          not null, primary key
+#  post_id      :integer          not null
+#  name         :string(256)      not null
+#  value        :text
+#  created_at   :datetime
+#  updated_at   :datetime
+#
+# Indexes
+#
+#  index_post_custom_fields_on_post_id_and_name  (post_id,name)
+#
diff --git a/db/migrate/20140425125742_add_custom_fields.rb b/db/migrate/20140425125742_add_custom_fields.rb
new file mode 100644
index 00000000000..3b6c542fde4
--- /dev/null
+++ b/db/migrate/20140425125742_add_custom_fields.rb
@@ -0,0 +1,28 @@
+class AddCustomFields < ActiveRecord::Migration
+  def change
+    create_table :category_custom_fields do |t|
+      t.integer :category_id, null: false
+      t.string :name, limit: 256, null: false
+      t.text :value
+      t.timestamps
+    end
+
+    create_table :group_custom_fields do |t|
+      t.integer :group_id, null: false
+      t.string :name, limit: 256, null: false
+      t.text :value
+      t.timestamps
+    end
+
+    create_table :post_custom_fields do |t|
+      t.integer :post_id, null: false
+      t.string :name, limit: 256, null: false
+      t.text :value
+      t.timestamps
+    end
+
+    add_index :category_custom_fields, [:category_id, :name]
+    add_index :group_custom_fields, [:group_id, :name]
+    add_index :post_custom_fields, [:post_id, :name]
+  end
+end
diff --git a/spec/models/category_spec.rb b/spec/models/category_spec.rb
index 5c10f2f2128..cb69dc6554d 100644
--- a/spec/models/category_spec.rb
+++ b/spec/models/category_spec.rb
@@ -133,6 +133,18 @@ describe Category do
     Fabricate(:category, name: "  blanks ").name.should == "blanks"
   end
 
+  it "has custom fields" do
+    category = Fabricate(:category, name: " music")
+    category.custom_fields["a"].should == nil
+
+    category.custom_fields["bob"] = "marley"
+    category.custom_fields["jack"] = "black"
+    category.save
+
+    category = Category.find(category.id)
+    category.custom_fields.should == {"bob" => "marley", "jack" => "black"}
+  end
+
   describe "short name" do
     let!(:category) { Fabricate(:category, name: 'xx') }
 
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 67675e71496..ca067f1c1d7 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -148,6 +148,19 @@ describe Group do
     GroupUser.count.should == original_count
   end
 
+
+  it "has custom fields" do
+    group = Fabricate(:group)
+    group.custom_fields["a"].should == nil
+
+    group.custom_fields["hugh"] = "jackman"
+    group.custom_fields["jack"] = "black"
+    group.save
+
+    group = Group.find(group.id)
+    group.custom_fields.should == {"hugh" => "jackman", "jack" => "black"}
+  end
+
   it "allows you to lookup a new group by name" do
     group = Fabricate(:group)
     group.id.should == Group[group.name].id
diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb
index 36d2984f8d4..fda07e82329 100644
--- a/spec/models/post_spec.rb
+++ b/spec/models/post_spec.rb
@@ -814,4 +814,16 @@ describe Post do
     end
   end
 
+  it "has custom fields" do
+    post = Fabricate(:post)
+    post.custom_fields["a"].should == nil
+
+    post.custom_fields["Tommy"] = "Hanks"
+    post.custom_fields["Vincent"] = "Vega"
+    post.save
+
+    post = Post.find(post.id)
+    post.custom_fields.should == {"Tommy" => "Hanks", "Vincent" => "Vega"}
+  end
+
 end

From 48f016c7f510bc51b21f3421c1c5debcce78fca3 Mon Sep 17 00:00:00 2001
From: Benjamin Kampmann <ben.kampmann@gmail.com>
Date: Fri, 25 Apr 2014 18:22:49 +0200
Subject: [PATCH 3/8] fix double save missing error by using copy not actual
 reference

---
 lib/concern/has_custom_fields.rb                  |  8 ++++++--
 spec/components/concern/has_custom_fields_spec.rb | 14 ++++++++++++++
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/lib/concern/has_custom_fields.rb b/lib/concern/has_custom_fields.rb
index 18273af10e0..aadd3b91f24 100644
--- a/lib/concern/has_custom_fields.rb
+++ b/lib/concern/has_custom_fields.rb
@@ -14,6 +14,10 @@ module Concern
       end
     end
 
+    def custom_fields=(data)
+      custom_fields.replace(data)
+    end
+
     protected
 
     def save_custom_fields
@@ -24,7 +28,7 @@ module Concern
           if dup[f.name] != f.value
             f.destroy
           else
-            dup.remove[f.name]
+            dup.delete(f.name)
           end
         end
 
@@ -32,7 +36,7 @@ module Concern
           _custom_fields.create(name: k, value: v)
         end
 
-        @custom_fields_orig = @custom_fields
+        @custom_fields_orig = dup
       end
     end
   end
diff --git a/spec/components/concern/has_custom_fields_spec.rb b/spec/components/concern/has_custom_fields_spec.rb
index 176e1c0cc96..c5c5f7a6018 100644
--- a/spec/components/concern/has_custom_fields_spec.rb
+++ b/spec/components/concern/has_custom_fields_spec.rb
@@ -51,6 +51,20 @@ describe Concern::HasCustomFields do
       test_item.custom_fields.should == {"jack" => "jill"}
     end
 
+    it "double save actually saves" do
+
+      test_item = TestItem.new
+      test_item.custom_fields = {"a" => "b"}
+      test_item.save
+
+      test_item.custom_fields["c"] = "d"
+      test_item.save
+
+      db_item = TestItem.find(test_item.id)
+      db_item.custom_fields.should == {"a" => "b", "c" => "d"}
+
+    end
+
 
     it "simple modifications don't interfere" do
       test_item = TestItem.new

From e502122c514ea3a511286b46615892fb749c03ce Mon Sep 17 00:00:00 2001
From: Benjamin Kampmann <ben.kampmann@gmail.com>
Date: Fri, 25 Apr 2014 18:24:22 +0200
Subject: [PATCH 4/8] Add Custom Fields on Topics

---
 app/models/topic.rb                           | 17 ++++++++++---
 app/models/topic_custom_field.rb              | 19 ++++++++++++++
 .../20140425135354_add_topic_custom_fields.rb | 21 ++++++++++++++++
 spec/models/topic_spec.rb                     | 25 +++++++++++++++++++
 4 files changed, 78 insertions(+), 4 deletions(-)
 create mode 100644 app/models/topic_custom_field.rb
 create mode 100644 db/migrate/20140425135354_add_topic_custom_fields.rb

diff --git a/app/models/topic.rb b/app/models/topic.rb
index 896c6d97200..0ff99cfc3c0 100644
--- a/app/models/topic.rb
+++ b/app/models/topic.rb
@@ -5,10 +5,12 @@ require_dependency 'rate_limiter'
 require_dependency 'text_sentinel'
 require_dependency 'text_cleaner'
 require_dependency 'archetype'
+require_dependency "concern/has_custom_fields"
 
 class Topic < ActiveRecord::Base
   include ActionView::Helpers::SanitizeHelper
   include RateLimiter::OnCreateRecord
+  include Concern::HasCustomFields
   include Trashable
   extend Forwardable
 
@@ -103,6 +105,7 @@ class Topic < ActiveRecord::Base
   attr_accessor :user_data
   attr_accessor :posters  # TODO: can replace with posters_summary once we remove old list code
   attr_accessor :topic_list
+  attr_accessor :meta_data
   attr_accessor :include_last_poster
 
   # The regular order
@@ -318,8 +321,16 @@ class Topic < ActiveRecord::Base
     topics.where("topics.id NOT IN (?)", featured_topic_ids)
   end
 
+  def meta_data=(data)
+    custom_fields.replace(data)
+  end
+
+  def meta_data
+    custom_fields
+  end
+
   def update_meta_data(data)
-    self.meta_data = (self.meta_data || {}).merge(data.stringify_keys)
+    custom_fields.update(data)
     save
   end
 
@@ -341,8 +352,7 @@ class Topic < ActiveRecord::Base
   end
 
   def meta_data_string(key)
-    return unless meta_data.present?
-    meta_data[key.to_s]
+    custom_fields[key.to_s]
   end
 
   def self.listable_count_per_day(sinceDaysAgo=30)
@@ -820,7 +830,6 @@ end
 #  archived                :boolean          default(FALSE), not null
 #  bumped_at               :datetime         not null
 #  has_summary             :boolean          default(FALSE), not null
-#  meta_data               :hstore
 #  vote_count              :integer          default(0), not null
 #  archetype               :string(255)      default("regular"), not null
 #  featured_user4_id       :integer
diff --git a/app/models/topic_custom_field.rb b/app/models/topic_custom_field.rb
new file mode 100644
index 00000000000..85db5aea30f
--- /dev/null
+++ b/app/models/topic_custom_field.rb
@@ -0,0 +1,19 @@
+class TopicCustomField < ActiveRecord::Base
+  belongs_to :topic
+end
+
+# == Schema Information
+#
+# Table name: topic_custom_fields
+#
+#  id           :integer          not null, primary key
+#  topic_id     :integer          not null
+#  name         :string(256)      not null
+#  value        :text
+#  created_at   :datetime
+#  updated_at   :datetime
+#
+# Indexes
+#
+#  index_topic_custom_fields_on_topic_id_and_name  (topic_id,name)
+#
diff --git a/db/migrate/20140425135354_add_topic_custom_fields.rb b/db/migrate/20140425135354_add_topic_custom_fields.rb
new file mode 100644
index 00000000000..4a059f8a628
--- /dev/null
+++ b/db/migrate/20140425135354_add_topic_custom_fields.rb
@@ -0,0 +1,21 @@
+class AddTopicCustomFields < ActiveRecord::Migration
+  def change
+    create_table :topic_custom_fields do |t|
+      t.integer :topic_id, null: false
+      t.string :name, limit: 256, null: false
+      t.text :value
+      t.timestamps
+    end
+
+    add_index :topic_custom_fields, [:topic_id, :name]
+
+    # migrate meta_data into custom fields
+    execute <<-SQL
+      INSERT INTO topic_custom_fields(topic_id, name, value)
+        SELECT id, (each(meta_data)).key, (each(meta_data)).value
+            FROM topics WHERE meta_data <> ''
+    SQL
+
+    remove_column :topics, :meta_data
+  end
+end
diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb
index 56eca86cde7..03bd04c3600 100644
--- a/spec/models/topic_spec.rb
+++ b/spec/models/topic_spec.rb
@@ -722,6 +722,21 @@ describe Topic do
 
       end
 
+      context 'new key' do
+        before do
+          topic.update_meta_data('other' => 'key')
+          topic.save!
+        end
+
+        it "can be loaded" do
+          Topic.find(topic.id).meta_data["other"].should == "key"
+        end
+
+        it "is in sync with custom_fields" do
+          Topic.find(topic.id).custom_fields["other"].should == "key"
+        end
+      end
+
 
     end
 
@@ -1380,7 +1395,17 @@ describe Topic do
       topic.stubs(:has_topic_embed?).returns(false)
       topic.expandable_first_post?.should be_false
     end
+  end
 
+  it "has custom fields" do
+    topic = Fabricate(:topic)
+    topic.custom_fields["a"].should == nil
 
+    topic.custom_fields["bob"] = "marley"
+    topic.custom_fields["jack"] = "black"
+    topic.save
+
+    topic = Topic.find(topic.id)
+    topic.custom_fields.should == {"bob" => "marley", "jack" => "black"}
   end
 end

From 1e70c3cbbd11943618cf8123ba998c3d2cf634fd Mon Sep 17 00:00:00 2001
From: Benjamin Kampmann <ben.kampmann@gmail.com>
Date: Fri, 25 Apr 2014 19:15:23 +0200
Subject: [PATCH 5/8] Add Support for Arrays to CustomFields

---
 lib/concern/has_custom_fields.rb              | 63 ++++++++++++++++---
 .../concern/has_custom_fields_spec.rb         | 43 +++++++++++++
 2 files changed, 97 insertions(+), 9 deletions(-)

diff --git a/lib/concern/has_custom_fields.rb b/lib/concern/has_custom_fields.rb
index aadd3b91f24..b0a61bd74b0 100644
--- a/lib/concern/has_custom_fields.rb
+++ b/lib/concern/has_custom_fields.rb
@@ -8,35 +8,80 @@ module Concern
     end
 
     def custom_fields
-      @custom_fields ||= begin
-        @custom_fields_orig = Hash[*_custom_fields.pluck(:name,:value).flatten]
-        @custom_fields_orig.dup
-      end
+      @custom_fields ||= refresh_custom_fields_from_db.dup
     end
 
     def custom_fields=(data)
       custom_fields.replace(data)
     end
 
+    def custom_fields_clean?
+      # Check whether the cached version has been
+      # changed on this model
+      !@custom_fields || @custom_fields_orig == @custom_fields
+    end
+
     protected
 
+    def refresh_custom_fields_from_db
+      target = Hash.new
+      _custom_fields.pluck(:name,:value).each do |key, value|
+        if target.has_key? key
+          if !target[key].is_a? Array
+            target[key] = [target[key]]
+          end
+          target[key] << value
+        else
+          target[key] = value
+        end
+      end
+      @custom_fields_orig = target
+      @custom_fields = @custom_fields_orig.dup
+    end
+
     def save_custom_fields
-      if @custom_fields && @custom_fields_orig != @custom_fields
+      if !custom_fields_clean?
         dup = @custom_fields.dup
 
+        array_fields = {}
+
         _custom_fields.each do |f|
-          if dup[f.name] != f.value
-            f.destroy
+          if dup[f.name].is_a? Array
+            # we need to collect Arrays fully before
+            # we can compare them
+            if !array_fields.has_key? f.name
+              array_fields[f.name] = [f]
+            else
+              array_fields[f.name] << f
+            end
           else
+            if dup[f.name] != f.value
+              f.destroy
+            else
+              dup.delete(f.name)
+            end
+          end
+        end
+
+        # let's iterate through our arrays and compare them
+        array_fields.each do |field_name, fields|
+          if fields.length == dup[field_name].length &&
+              fields.map{|f| f.value} == dup[field_name]
             dup.delete(f.name)
+          else
+            fields.each{|f| f.destroy }
           end
         end
 
         dup.each do |k,v|
-          _custom_fields.create(name: k, value: v)
+          if v.is_a? Array
+            v.each {|subv| _custom_fields.create(name: k, value: subv)}
+          else
+            _custom_fields.create(name: k, value: v)
+          end
         end
 
-        @custom_fields_orig = dup
+        refresh_custom_fields_from_db
       end
     end
   end
diff --git a/spec/components/concern/has_custom_fields_spec.rb b/spec/components/concern/has_custom_fields_spec.rb
index c5c5f7a6018..9f8ae26aa5b 100644
--- a/spec/components/concern/has_custom_fields_spec.rb
+++ b/spec/components/concern/has_custom_fields_spec.rb
@@ -35,6 +35,7 @@ describe Concern::HasCustomFields do
 
       test_item.custom_fields["bob"] = "marley"
       test_item.custom_fields["jack"] = "black"
+
       test_item.save
 
       test_item = TestItem.find(test_item.id)
@@ -51,6 +52,21 @@ describe Concern::HasCustomFields do
       test_item.custom_fields.should == {"jack" => "jill"}
     end
 
+    it "casts integers to string without error" do
+      test_item = TestItem.new
+      test_item.custom_fields["a"].should == nil
+      test_item.custom_fields["a"] = 0
+
+      test_item.custom_fields["a"].should == 0
+      test_item.save
+
+      # should be casted right after saving
+      test_item.custom_fields["a"].should == "0"
+
+      test_item = TestItem.find(test_item.id)
+      test_item.custom_fields["a"].should == "0"
+    end
+
     it "double save actually saves" do
 
       test_item = TestItem.new
@@ -66,6 +82,33 @@ describe Concern::HasCustomFields do
     end
 
 
+    it "handles arrays properly" do
+
+      test_item = TestItem.new
+      test_item.custom_fields = {"a" => ["b", "c", "d"]}
+      test_item.save
+
+      db_item = TestItem.find(test_item.id)
+      db_item.custom_fields.should == {"a" => ["b", "c", "d"]}
+
+      db_item.custom_fields["a"] = ["c", "d"]
+      db_item.save
+      db_item.custom_fields.should == {"a" => ["c", "d"]}
+
+    end
+
+    it "casts integers in arrays properly without error" do
+
+      test_item = TestItem.new
+      test_item.custom_fields = {"a" => ["b", 10, "d"]}
+      test_item.save
+      test_item.custom_fields.should == {"a" => ["b", "10", "d"]}
+
+      db_item = TestItem.find(test_item.id)
+      db_item.custom_fields.should == {"a" => ["b", "10", "d"]}
+
+    end
+
     it "simple modifications don't interfere" do
       test_item = TestItem.new
 

From 0cf07d41ae0b5a5e1a27f70f914286f83f57bade Mon Sep 17 00:00:00 2001
From: Benjamin Kampmann <ben.kampmann@gmail.com>
Date: Mon, 28 Apr 2014 10:31:51 +0200
Subject: [PATCH 6/8] Move Concern from lib into app/models. refs #2279

---
 app/models/category.rb                        |  4 +-
 app/models/concerns/has_custom_fields.rb      | 87 ++++++++++++++++++
 app/models/group.rb                           |  4 +-
 app/models/post.rb                            |  3 +-
 app/models/topic.rb                           |  3 +-
 app/models/user.rb                            |  3 +-
 lib/concern/has_custom_fields.rb              | 88 -------------------
 .../concern/has_custom_fields_spec.rb         |  5 +-
 8 files changed, 94 insertions(+), 103 deletions(-)
 create mode 100644 app/models/concerns/has_custom_fields.rb
 delete mode 100644 lib/concern/has_custom_fields.rb

diff --git a/app/models/category.rb b/app/models/category.rb
index 4bd71f5be4b..2ef1a7501e1 100644
--- a/app/models/category.rb
+++ b/app/models/category.rb
@@ -1,9 +1,7 @@
-require_dependency "concern/has_custom_fields"
-
 class Category < ActiveRecord::Base
 
   include Positionable
-  include Concern::HasCustomFields
+  include HasCustomFields
 
   belongs_to :topic, dependent: :destroy
   belongs_to :topic_only_relative_url,
diff --git a/app/models/concerns/has_custom_fields.rb b/app/models/concerns/has_custom_fields.rb
new file mode 100644
index 00000000000..c763db04d7d
--- /dev/null
+++ b/app/models/concerns/has_custom_fields.rb
@@ -0,0 +1,87 @@
+
+module HasCustomFields
+  extend ActiveSupport::Concern
+
+  included do
+    has_many :_custom_fields, dependent: :destroy, :class_name => "#{name}CustomField"
+    after_save :save_custom_fields
+  end
+
+  def custom_fields
+    @custom_fields ||= refresh_custom_fields_from_db.dup
+  end
+
+  def custom_fields=(data)
+    custom_fields.replace(data)
+  end
+
+  def custom_fields_clean?
+    # Check whether the cached version has been
+    # changed on this model
+    !@custom_fields || @custom_fields_orig == @custom_fields
+  end
+
+  protected
+
+  def refresh_custom_fields_from_db
+    target = Hash.new
+    _custom_fields.pluck(:name,:value).each do |key, value|
+      if target.has_key? key
+        if !target[key].is_a? Array
+          target[key] = [target[key]]
+        end
+        target[key] << value
+      else
+        target[key] = value
+      end
+    end
+    @custom_fields_orig = target
+    @custom_fields = @custom_fields_orig.dup
+  end
+
+  def save_custom_fields
+    if !custom_fields_clean?
+      dup = @custom_fields.dup
+
+      array_fields = {}
+
+      _custom_fields.each do |f|
+        if dup[f.name].is_a? Array
+          # we need to collect Arrays fully before
+          # we can compare them
+          if !array_fields.has_key? f.name
+            array_fields[f.name] = [f]
+          else
+            array_fields[f.name] << f
+          end
+        else
+          if dup[f.name] != f.value
+            f.destroy
+          else
+            dup.delete(f.name)
+          end
+        end
+      end
+
+      # let's iterate through our arrays and compare them
+      array_fields.each do |field_name, fields|
+        if fields.length == dup[field_name].length &&
+            fields.map{|f| f.value} == dup[field_name]
+          dup.delete(f.name)
+        else
+          fields.each{|f| f.destroy }
+        end
+      end
+
+      dup.each do |k,v|
+        if v.is_a? Array
+          v.each {|subv| _custom_fields.create(name: k, value: subv)}
+        else
+          _custom_fields.create(name: k, value: v)
+        end
+      end
+
+      refresh_custom_fields_from_db
+    end
+  end
+end
\ No newline at end of file
diff --git a/app/models/group.rb b/app/models/group.rb
index 35933da1012..e5cd9dc8a90 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -1,7 +1,5 @@
-require_dependency "concern/has_custom_fields"
-
 class Group < ActiveRecord::Base
-  include Concern::HasCustomFields
+  include HasCustomFields
 
   has_many :category_groups
   has_many :group_users, dependent: :destroy
diff --git a/app/models/post.rb b/app/models/post.rb
index fc38fc40798..cdba83b5c2e 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -6,7 +6,6 @@ require_dependency 'enum'
 require_dependency 'post_analyzer'
 require_dependency 'validators/post_validator'
 require_dependency 'plugin/filter'
-require_dependency "concern/has_custom_fields"
 
 require 'archetype'
 require 'digest/sha1'
@@ -14,7 +13,7 @@ require 'digest/sha1'
 class Post < ActiveRecord::Base
   include RateLimiter::OnCreateRecord
   include Trashable
-  include Concern::HasCustomFields
+  include HasCustomFields
 
   rate_limit
   rate_limit :limit_posts_per_day
diff --git a/app/models/topic.rb b/app/models/topic.rb
index 0ff99cfc3c0..9360b98477d 100644
--- a/app/models/topic.rb
+++ b/app/models/topic.rb
@@ -5,12 +5,11 @@ require_dependency 'rate_limiter'
 require_dependency 'text_sentinel'
 require_dependency 'text_cleaner'
 require_dependency 'archetype'
-require_dependency "concern/has_custom_fields"
 
 class Topic < ActiveRecord::Base
   include ActionView::Helpers::SanitizeHelper
   include RateLimiter::OnCreateRecord
-  include Concern::HasCustomFields
+  include HasCustomFields
   include Trashable
   extend Forwardable
 
diff --git a/app/models/user.rb b/app/models/user.rb
index a7f02c5af86..962d2d3bcde 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -8,12 +8,11 @@ require_dependency 'post_destroyer'
 require_dependency 'user_name_suggester'
 require_dependency 'pretty_text'
 require_dependency 'url_helper'
-require_dependency 'concern/has_custom_fields'
 
 class User < ActiveRecord::Base
   include Roleable
   include UrlHelper
-  include Concern::HasCustomFields
+  include HasCustomFields
 
   has_many :posts
   has_many :notifications, dependent: :destroy
diff --git a/lib/concern/has_custom_fields.rb b/lib/concern/has_custom_fields.rb
deleted file mode 100644
index b0a61bd74b0..00000000000
--- a/lib/concern/has_custom_fields.rb
+++ /dev/null
@@ -1,88 +0,0 @@
-module Concern
-  module HasCustomFields
-    extend ActiveSupport::Concern
-
-    included do
-      has_many :_custom_fields, dependent: :destroy, :class_name => "#{name}CustomField"
-      after_save :save_custom_fields
-    end
-
-    def custom_fields
-      @custom_fields ||= refresh_custom_fields_from_db.dup
-    end
-
-    def custom_fields=(data)
-      custom_fields.replace(data)
-    end
-
-    def custom_fields_clean?
-      # Check whether the cached version has been
-      # changed on this model
-      !@custom_fields || @custom_fields_orig == @custom_fields
-    end
-
-    protected
-
-    def refresh_custom_fields_from_db
-      target = Hash.new
-      _custom_fields.pluck(:name,:value).each do |key, value|
-        if target.has_key? key
-          if !target[key].is_a? Array
-            target[key] = [target[key]]
-          end
-          target[key] << value
-        else
-          target[key] = value
-        end
-      end
-      @custom_fields_orig = target
-      @custom_fields = @custom_fields_orig.dup
-    end
-
-    def save_custom_fields
-      if !custom_fields_clean?
-        dup = @custom_fields.dup
-
-        array_fields = {}
-
-        _custom_fields.each do |f|
-          if dup[f.name].is_a? Array
-            # we need to collect Arrays fully before
-            # we can compare them
-            if !array_fields.has_key? f.name
-              array_fields[f.name] = [f]
-            else
-              array_fields[f.name] << f
-            end
-          else
-            if dup[f.name] != f.value
-              f.destroy
-            else
-              dup.delete(f.name)
-            end
-          end
-        end
-
-        # let's iterate through our arrays and compare them
-        array_fields.each do |field_name, fields|
-          if fields.length == dup[field_name].length &&
-              fields.map{|f| f.value} == dup[field_name]
-            dup.delete(f.name)
-          else
-            fields.each{|f| f.destroy }
-          end
-        end
-
-        dup.each do |k,v|
-          if v.is_a? Array
-            v.each {|subv| _custom_fields.create(name: k, value: subv)}
-          else
-            _custom_fields.create(name: k, value: v)
-          end
-        end
-
-        refresh_custom_fields_from_db
-      end
-    end
-  end
-end
\ No newline at end of file
diff --git a/spec/components/concern/has_custom_fields_spec.rb b/spec/components/concern/has_custom_fields_spec.rb
index 9f8ae26aa5b..291af92cb26 100644
--- a/spec/components/concern/has_custom_fields_spec.rb
+++ b/spec/components/concern/has_custom_fields_spec.rb
@@ -1,7 +1,6 @@
 require "spec_helper"
-require_dependency "concern/has_custom_fields"
 
-describe Concern::HasCustomFields do
+describe HasCustomFields do
 
   context "custom_fields" do
     before do
@@ -10,7 +9,7 @@ describe Concern::HasCustomFields do
       Topic.exec_sql("create temporary table test_item_custom_fields(id SERIAL primary key, test_item_id int, name varchar(256) not null, value text)")
 
       class TestItem < ActiveRecord::Base
-        include Concern::HasCustomFields
+        include HasCustomFields
       end
 
       class TestItemCustomField < ActiveRecord::Base

From 230453b411863a3ee8c766c73e865763e84a26f4 Mon Sep 17 00:00:00 2001
From: Benjamin Kampmann <ben.kampmann@gmail.com>
Date: Mon, 28 Apr 2014 10:37:34 +0200
Subject: [PATCH 7/8] use more explicit naming to prevent name clashes. fixes
 build.

---
 .../concern/has_custom_fields_spec.rb         | 49 ++++++++++---------
 1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/spec/components/concern/has_custom_fields_spec.rb b/spec/components/concern/has_custom_fields_spec.rb
index 291af92cb26..7f81dceb5ea 100644
--- a/spec/components/concern/has_custom_fields_spec.rb
+++ b/spec/components/concern/has_custom_fields_spec.rb
@@ -1,34 +1,35 @@
 require "spec_helper"
 
+
 describe HasCustomFields do
 
   context "custom_fields" do
     before do
 
-      Topic.exec_sql("create temporary table test_items(id SERIAL primary key)")
-      Topic.exec_sql("create temporary table test_item_custom_fields(id SERIAL primary key, test_item_id int, name varchar(256) not null, value text)")
+      Topic.exec_sql("create temporary table custom_fields_test_items(id SERIAL primary key)")
+      Topic.exec_sql("create temporary table custom_fields_test_item_custom_fields(id SERIAL primary key, custom_fields_test_item_id int, name varchar(256) not null, value text)")
 
-      class TestItem < ActiveRecord::Base
+      class CustomFieldsTestItem < ActiveRecord::Base
         include HasCustomFields
       end
 
-      class TestItemCustomField < ActiveRecord::Base
-        belongs_to :test_item
+      class CustomFieldsTestItemCustomField < ActiveRecord::Base
+        belongs_to :custom_fields_test_item
       end
     end
 
     after do
-      Topic.exec_sql("drop table test_items")
-      Topic.exec_sql("drop table test_item_custom_fields")
+      Topic.exec_sql("drop table custom_fields_test_items")
+      Topic.exec_sql("drop table custom_fields_test_item_custom_fields")
 
       # import is making my life hard, we need to nuke this out of orbit
       des = ActiveSupport::DescendantsTracker.class_variable_get :@@direct_descendants
-      des[ActiveRecord::Base].delete(TestItem)
-      des[ActiveRecord::Base].delete(TestItemCustomField)
+      des[ActiveRecord::Base].delete(CustomFieldsTestItem)
+      des[ActiveRecord::Base].delete(CustomFieldsTestItemCustomField)
     end
 
     it "simple modification of custom fields" do
-      test_item = TestItem.new
+      test_item = CustomFieldsTestItem.new
 
       test_item.custom_fields["a"].should == nil
 
@@ -37,7 +38,7 @@ describe HasCustomFields do
 
       test_item.save
 
-      test_item = TestItem.find(test_item.id)
+      test_item = CustomFieldsTestItem.find(test_item.id)
 
       test_item.custom_fields["bob"].should == "marley"
       test_item.custom_fields["jack"].should == "black"
@@ -46,13 +47,13 @@ describe HasCustomFields do
       test_item.custom_fields["jack"] = "jill"
 
       test_item.save
-      test_item = TestItem.find(test_item.id)
+      test_item = CustomFieldsTestItem.find(test_item.id)
 
       test_item.custom_fields.should == {"jack" => "jill"}
     end
 
     it "casts integers to string without error" do
-      test_item = TestItem.new
+      test_item = CustomFieldsTestItem.new
       test_item.custom_fields["a"].should == nil
       test_item.custom_fields["a"] = 0
 
@@ -62,20 +63,20 @@ describe HasCustomFields do
       # should be casted right after saving
       test_item.custom_fields["a"].should == "0"
 
-      test_item = TestItem.find(test_item.id)
+      test_item = CustomFieldsTestItem.find(test_item.id)
       test_item.custom_fields["a"].should == "0"
     end
 
     it "double save actually saves" do
 
-      test_item = TestItem.new
+      test_item = CustomFieldsTestItem.new
       test_item.custom_fields = {"a" => "b"}
       test_item.save
 
       test_item.custom_fields["c"] = "d"
       test_item.save
 
-      db_item = TestItem.find(test_item.id)
+      db_item = CustomFieldsTestItem.find(test_item.id)
       db_item.custom_fields.should == {"a" => "b", "c" => "d"}
 
     end
@@ -83,11 +84,11 @@ describe HasCustomFields do
 
     it "handles arrays properly" do
 
-      test_item = TestItem.new
+      test_item = CustomFieldsTestItem.new
       test_item.custom_fields = {"a" => ["b", "c", "d"]}
       test_item.save
 
-      db_item = TestItem.find(test_item.id)
+      db_item = CustomFieldsTestItem.find(test_item.id)
       db_item.custom_fields.should == {"a" => ["b", "c", "d"]}
 
       db_item.custom_fields["a"] = ["c", "d"]
@@ -98,18 +99,18 @@ describe HasCustomFields do
 
     it "casts integers in arrays properly without error" do
 
-      test_item = TestItem.new
+      test_item = CustomFieldsTestItem.new
       test_item.custom_fields = {"a" => ["b", 10, "d"]}
       test_item.save
       test_item.custom_fields.should == {"a" => ["b", "10", "d"]}
 
-      db_item = TestItem.find(test_item.id)
+      db_item = CustomFieldsTestItem.find(test_item.id)
       db_item.custom_fields.should == {"a" => ["b", "10", "d"]}
 
     end
 
     it "simple modifications don't interfere" do
-      test_item = TestItem.new
+      test_item = CustomFieldsTestItem.new
 
       test_item.custom_fields["a"].should == nil
 
@@ -117,7 +118,7 @@ describe HasCustomFields do
       test_item.custom_fields["jack"] = "black"
       test_item.save
 
-      test_item2 = TestItem.new
+      test_item2 = CustomFieldsTestItem.new
 
       test_item2.custom_fields["x"].should == nil
 
@@ -125,8 +126,8 @@ describe HasCustomFields do
       test_item2.custom_fields["de"] = "la playa"
       test_item2.save
 
-      test_item = TestItem.find(test_item.id)
-      test_item2 = TestItem.find(test_item2.id)
+      test_item = CustomFieldsTestItem.find(test_item.id)
+      test_item2 = CustomFieldsTestItem.find(test_item2.id)
 
       test_item.custom_fields.should == {"jack" => "black", "bob" => "marley"}
       test_item2.custom_fields.should == {"sixto" => "rodriguez", "de" => "la playa"}

From f7577068613dd162c55929a0f947985bcf0102d4 Mon Sep 17 00:00:00 2001
From: Benjamin Kampmann <ben.kampmann@gmail.com>
Date: Tue, 29 Apr 2014 19:23:13 +0200
Subject: [PATCH 8/8] Ensure Reload reloads custom_fields, too

---
 app/models/concerns/has_custom_fields.rb      |  6 ++++++
 .../concern/has_custom_fields_spec.rb         | 20 +++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/app/models/concerns/has_custom_fields.rb b/app/models/concerns/has_custom_fields.rb
index c763db04d7d..f6b9261984e 100644
--- a/app/models/concerns/has_custom_fields.rb
+++ b/app/models/concerns/has_custom_fields.rb
@@ -7,6 +7,12 @@ module HasCustomFields
     after_save :save_custom_fields
   end
 
+  def reload(options = nil)
+    @custom_fields = nil
+    @custom_fields_orig = nil
+    super
+  end
+
   def custom_fields
     @custom_fields ||= refresh_custom_fields_from_db.dup
   end
diff --git a/spec/components/concern/has_custom_fields_spec.rb b/spec/components/concern/has_custom_fields_spec.rb
index 7f81dceb5ea..e37e9a72aa2 100644
--- a/spec/components/concern/has_custom_fields_spec.rb
+++ b/spec/components/concern/has_custom_fields_spec.rb
@@ -67,6 +67,26 @@ describe HasCustomFields do
       test_item.custom_fields["a"].should == "0"
     end
 
+    it "reload loads from database" do
+      test_item = CustomFieldsTestItem.new
+      test_item.custom_fields["a"] = 0
+
+      test_item.custom_fields["a"].should == 0
+      test_item.save
+
+      # should be casted right after saving
+      test_item.custom_fields["a"].should == "0"
+
+      CustomFieldsTestItem.exec_sql("UPDATE custom_fields_test_item_custom_fields SET value='1' WHERE custom_fields_test_item_id=? AND name='a'", test_item.id)
+
+      # still the same, did not load
+      test_item.custom_fields["a"].should == "0"
+
+      # refresh loads from database
+      test_item.reload.custom_fields["a"].should == "1"
+      test_item.custom_fields["a"].should == "1"
+
+    end
     it "double save actually saves" do
 
       test_item = CustomFieldsTestItem.new