diff --git a/app/assets/javascripts/app/collections/pods.js b/app/assets/javascripts/app/collections/pods.js index 18573be99..1264d09c4 100644 --- a/app/assets/javascripts/app/collections/pods.js +++ b/app/assets/javascripts/app/collections/pods.js @@ -3,7 +3,8 @@ app.collections.Pods = Backbone.Collection.extend({ model: app.models.Pod, comparator: function(model) { - return model.get("host").toLowerCase(); + var host = model.get("host") || ""; + return host.toLowerCase(); } }); // @license-end diff --git a/app/assets/javascripts/app/pages/admin_pods.js b/app/assets/javascripts/app/pages/admin_pods.js index b91204e69..4c4b14dd7 100644 --- a/app/assets/javascripts/app/pages/admin_pods.js +++ b/app/assets/javascripts/app/pages/admin_pods.js @@ -3,6 +3,8 @@ app.pages.AdminPods = app.views.Base.extend({ templateName: "pod_table", + tooltipSelector: "th i", + initialize: function() { this.pods = new app.collections.Pods(app.parsePreload("pods")); this.rows = []; // contains the table row views diff --git a/app/assets/javascripts/app/views/pod_entry_view.js b/app/assets/javascripts/app/views/pod_entry_view.js index 01ca99490..8e08c9771 100644 --- a/app/assets/javascripts/app/views/pod_entry_view.js +++ b/app/assets/javascripts/app/views/pod_entry_view.js @@ -10,6 +10,8 @@ app.views.PodEntry = app.views.Base.extend({ "click .recheck": "recheckPod" }, + tooltipSelector: ".ssl-status i, .actions i", + className: function() { if( this.model.get("offline") ) { return "bg-danger"; } if( this.model.get("status")==="version_failed" ) { return "bg-warning"; } diff --git a/app/assets/templates/pod_table_entry_tpl.jst.hbs b/app/assets/templates/pod_table_entry_tpl.jst.hbs index a7ebfe6ae..9daa5b331 100644 --- a/app/assets/templates/pod_table_entry_tpl.jst.hbs +++ b/app/assets/templates/pod_table_entry_tpl.jst.hbs @@ -1,5 +1,5 @@ - + {{#if ssl}} {{else}} diff --git a/app/models/pod.rb b/app/models/pod.rb index 1d57aec58..c835fbce7 100644 --- a/app/models/pod.rb +++ b/app/models/pod.rb @@ -44,7 +44,7 @@ class Pod < ActiveRecord::Base end def check_all! - Pod.find_in_batches(batch_size: 20).each(&:test_connection!) + Pod.find_in_batches(batch_size: 20) {|batch| batch.each(&:test_connection!) } end end diff --git a/db/migrate/20150828132451_remove_duplicate_and_empty_pods.rb b/db/migrate/20150828132451_remove_duplicate_and_empty_pods.rb new file mode 100644 index 000000000..e93acf722 --- /dev/null +++ b/db/migrate/20150828132451_remove_duplicate_and_empty_pods.rb @@ -0,0 +1,24 @@ +class RemoveDuplicateAndEmptyPods < ActiveRecord::Migration + def up + remove_dupes + remove_empty_or_nil + + add_index :pods, :host, unique: true, length: 190 # =190*4 for utf8mb4 + end + + def down + remove_index :pods, :host + end + + private + + def remove_dupes + duplicates = Pod.group(:host).count.select {|_, v| v > 1 }.keys + ids = duplicates.flat_map {|pod| Pod.where(host: pod).order(created_at: :asc).pluck(:id).tap(&:shift) } + Pod.where(id: ids).destroy_all + end + + def remove_empty_or_nil + Pod.where(host: [nil, ""]).destroy_all + end +end diff --git a/db/schema.rb b/db/schema.rb index 284aa9957..f2afae365 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150731123114) do +ActiveRecord::Schema.define(version: 20150828132451) do create_table "account_deletions", force: :cascade do |t| t.string "diaspora_handle", limit: 255 @@ -319,6 +319,7 @@ ActiveRecord::Schema.define(version: 20150731123114) do end add_index "pods", ["checked_at"], name: "index_pods_on_checked_at", using: :btree + add_index "pods", ["host"], name: "index_pods_on_host", unique: true, length: {"host"=>190}, using: :btree add_index "pods", ["offline_since"], name: "index_pods_on_offline_since", using: :btree add_index "pods", ["status"], name: "index_pods_on_status", using: :btree diff --git a/spec/factories.rb b/spec/factories.rb index d3470e3be..1f99c0f0b 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -211,7 +211,7 @@ FactoryGirl.define do end factory :pod do - host "pod.example.com" + sequence(:host) {|n| "pod#{n}.example#{r_str}.com" } ssl true end diff --git a/spec/javascripts/app/collections/pods_spec.js b/spec/javascripts/app/collections/pods_spec.js new file mode 100644 index 000000000..97bb29619 --- /dev/null +++ b/spec/javascripts/app/collections/pods_spec.js @@ -0,0 +1,16 @@ + +describe("app.collections.Pods", function() { + describe("#comparator", function() { + it("should handle empty hostnames", function() { + var collection = new app.collections.Pods([ + {id: 1}, + {id: 2, host: "zzz.zz"}, + {id: 3, host: "aaa.aa"}, + {id: 4, host: ""} + ]); + expect(collection.length).toBe(4); + expect(collection.first().get("host")).toBeFalsy(); // also empty string + expect(collection.last().get("host")).toBe("zzz.zz"); + }); + }); +}); diff --git a/spec/migrations/remove_duplicate_and_empty_pods_spec.rb b/spec/migrations/remove_duplicate_and_empty_pods_spec.rb new file mode 100644 index 000000000..fc26b9637 --- /dev/null +++ b/spec/migrations/remove_duplicate_and_empty_pods_spec.rb @@ -0,0 +1,64 @@ + +require "spec_helper" +require Rails.root.join("db/migrate/20150828132451_remove_duplicate_and_empty_pods.rb") + +describe RemoveDuplicateAndEmptyPods do + self.use_transactional_fixtures = false + + before do + @previous_migration = 20_150_731_123_114 + @my_migration = 20_150_828_132_451 + Pod.delete_all + end + + after :all do + migrate_to(nil) # up + Pod.delete_all # cleanup manually, transactions are disabled + end + + def migrate_to(version) + ActiveRecord::Migrator.migrate(Rails.root.join("db", "migrate"), version) + end + + describe "#up" do + before do + migrate_to(@previous_migration) + + FactoryGirl.create(:pod, host: nil) + FactoryGirl.create(:pod, host: "") + 4.times { FactoryGirl.create(:pod, host: "aaa.aa") } + end + + it "removes duplicates" do + expect(Pod.where(host: "aaa.aa").count).to eql(4) + migrate_to(@my_migration) + expect(Pod.where(host: "aaa.aa").count).to eql(1) + end + + it "removes empty hostnames" do + expect(Pod.where(host: [nil, ""]).count).to eql(2) + migrate_to(@my_migration) + expect(Pod.where(host: [nil, ""]).count).to eql(0) + end + + it "adds an index on the host column" do + expect { + migrate_to(@my_migration) + Pod.reset_column_information + }.to change { Pod.connection.indexes(Pod.table_name).count }.by(1) + end + end + + describe "#down" do + before do + migrate_to(@my_migration) + end + + it "removes the index on the host column" do + expect { + migrate_to(@previous_migration) + Pod.reset_column_information + }.to change { Pod.connection.indexes(Pod.table_name).count }.by(-1) + end + end +end diff --git a/spec/models/pod_spec.rb b/spec/models/pod_spec.rb index fd9d148e3..a1e851d26 100644 --- a/spec/models/pod_spec.rb +++ b/spec/models/pod_spec.rb @@ -20,7 +20,7 @@ describe Pod, type: :model do expect(pod).to receive(:test_connection!) end end - allow(Pod).to receive(:find_in_batches) { @pods } + allow(Pod).to receive(:find_in_batches).and_yield(@pods) end it "calls #test_connection! on every pod" do diff --git a/spec/workers/recurring_pod_check_spec.rb b/spec/workers/recurring_pod_check_spec.rb index b7f240f1e..fec3b7f24 100644 --- a/spec/workers/recurring_pod_check_spec.rb +++ b/spec/workers/recurring_pod_check_spec.rb @@ -4,11 +4,11 @@ require "spec_helper" describe Workers::RecurringPodCheck do before do @pods = (0..4).map do - FactoryGirl.create(:pod).tap { |pod| + FactoryGirl.build(:pod).tap { |pod| expect(pod).to receive(:test_connection!) } end - allow(Pod).to receive(:find_in_batches) { @pods } + allow(Pod).to receive(:find_in_batches).and_yield(@pods) end it "performs a connection test on all existing pods" do