Merge pull request #6727 from SuperTux88/improve-pod-check

Improve pod check

Changelog.md

@@ -91,6 +91,7 @@ Contributions are very welcome, the hard work is done!
 * Dropped `parent_author_signature` from relayables [#6586](https://github.com/diaspora/diaspora/pull/6586)
 * Attached ShareVisibilities to the User, not the Contact [#6723](https://github.com/diaspora/diaspora/pull/6723)
 * Refactor mentions input, now based on typeahead.js [#6728](https://github.com/diaspora/diaspora/pull/6728)
+* Optimized the pod up checks [#6727](https://github.com/diaspora/diaspora/pull/6727)
 
 ## Bug fixes
 * Destroy Participation when removing interactions with a post [#5852](https://github.com/diaspora/diaspora/pull/5852)

app/assets/javascripts/app/pages/admin_pods.js

@@ -34,6 +34,11 @@ app.pages.AdminPods = app.views.Base.extend({
         .append(Diaspora.I18n.t("admin.pods.unchecked", {count: gon.uncheckedCount}));
       msgs.appendChild(unchecked[0]);
     }
+    if( gon.versionFailedCount && gon.versionFailedCount > 0 ) {
+      var versionFailed = $("<div class='alert alert-warning' />")
+          .append(Diaspora.I18n.t("admin.pods.version_failed", {count: gon.versionFailedCount}));
+      msgs.appendChild(versionFailed[0]);
+    }
     if( gon.errorCount && gon.errorCount > 0 ) {
       var errors = $("<div class='alert alert-danger' />")
         .append(Diaspora.I18n.t("admin.pods.errors", {count: gon.errorCount}));

app/assets/javascripts/app/views/pod_entry_view.js

@@ -31,7 +31,8 @@ app.views.PodEntry = app.views.Base.extend({
       has_no_errors: (this.model.get("status")==="no_errors"),
       has_errors: (this.model.get("status")!=="no_errors"),
       status_text: Diaspora.I18n.t("admin.pods.states."+this.model.get("status")),
-      pod_url: (this.model.get("ssl") ? "https" : "http") + "://" + this.model.get("host"),
+      pod_url: (this.model.get("ssl") ? "https" : "http") + "://" + this.model.get("host") +
+                 (this.model.get("port") ? ":" + this.model.get("port") : ""),
       response_time_fmt: this._fmtResponseTime()
       /* jshint camelcase: true */
     });

app/assets/stylesheets/admin.scss

@@ -39,6 +39,12 @@
 /** pod list **/
 
 #pod-list {
+  .pod-title {
+    max-width: 200px;
+    overflow: hidden;
+    text-overflow: ellipsis;
+  }
+
   th.added,
   td.added,
   td.actions { white-space: nowrap; }

app/assets/templates/pod_table_entry_tpl.jst.hbs

@@ -7,7 +7,7 @@
   {{/if}}
   </i>
 </td>
-<td>{{host}}</td>
+<td class="pod-title" title="{{host}}">{{host}}</td>
 <td class="added">
   <small><time datetime="{{created_at}}" title="{{localTime created_at}}" /></small>
 </td>

app/controllers/admin/pods_controller.rb

@@ -10,6 +10,7 @@ module Admin
         format.html do
           gon.preloads[:pods] = pods_json
           gon.unchecked_count = Pod.unchecked.count
+          gon.version_failed_count = Pod.version_failed.count
           gon.error_count = Pod.check_failed.count
 
           render "admins/pods"

app/models/person.rb

@@ -50,15 +50,14 @@ class Person < ActiveRecord::Base
   has_many :roles
 
   belongs_to :owner, :class_name => 'User'
+  belongs_to :pod
 
   has_many :notification_actors
   has_many :notifications, :through => :notification_actors
 
   has_many :mentions, :dependent => :destroy
 
-  before_validation :clean_url
+  validate :owner_xor_pod
-
-  validates :url, :presence => true
   validates :profile, :presence => true
   validates :serialized_public_key, :presence => true
   validates :diaspora_handle, :uniqueness => true
@@ -205,8 +204,6 @@ class Person < ActiveRecord::Base
 
   def url
     url_to "/"
-  rescue
-    self[:url]
   end
 
   def profile_url
@@ -290,23 +287,6 @@ class Person < ActiveRecord::Base
     json
   end
 
-  # Update an array of people given a url, and set it as the new destination_url
-  # @param people [Array<People>]
-  # @param url [String]
-  def self.url_batch_update(people, url)
-    people.each do |person|
-      person.update_url(url)
-    end
-  end
-
-  # @param person [Person]
-  # @param url [String]
-  def update_url(url)
-    @uri = URI.parse(url)
-    @uri.path = "/"
-    update_attributes(:url => @uri.to_s)
-  end
-
   def lock_access!
     self.closed_account = true
     self.save
@@ -317,27 +297,12 @@ class Person < ActiveRecord::Base
     self
   end
 
-  protected
-
-  def clean_url
-    if self.url
-      self.url = 'http://' + self.url unless self.url.match(/https?:\/\//)
-      self.url = self.url + '/' if self.url[-1, 1] != '/'
-    end
-  end
-
   private
 
-  # @return [URI]
-  def uri
-    @uri ||= URI.parse(self[:url])
-    @uri.dup
-  end
-
   # @param path [String]
   # @return [String]
   def url_to(path)
-    uri.tap {|uri| uri.path = path }.to_s
+    local? ? AppConfig.url_to(path) : pod.url_to(path)
   end
 
   def fix_profile
@@ -345,4 +310,8 @@ class Person < ActiveRecord::Base
     DiasporaFederation::Discovery::Discovery.new(diaspora_handle).fetch_and_save
     reload
   end
+
+  def owner_xor_pod
+    errors.add(:base, "Specify an owner or a pod, not both") unless owner.blank? ^ pod.blank?
+  end
 end

app/models/pod.rb

@@ -19,18 +19,21 @@ class Pod < ActiveRecord::Base
     ConnectionTester::NodeInfoFailure => :version_failed
   }
 
+  DEFAULT_PORTS = [URI::HTTP::DEFAULT_PORT, URI::HTTPS::DEFAULT_PORT]
+
+  has_many :people
+
   scope :check_failed, lambda {
-    where(arel_table[:status].gt(Pod.statuses[:no_errors]))
+    where(arel_table[:status].gt(Pod.statuses[:no_errors])).where.not(status: Pod.statuses[:version_failed])
   }
 
   class << self
     def find_or_create_by(opts) # Rename this method to not override an AR method
-      u = URI.parse(opts.fetch(:url))
+      uri = URI.parse(opts.fetch(:url))
-      find_or_initialize_by(host: u.host).tap do |pod|
+      port = DEFAULT_PORTS.include?(uri.port) ? nil : uri.port
-        unless pod.persisted?
+      find_or_initialize_by(host: uri.host, port: port).tap do |pod|
-          pod.ssl = (u.scheme == "https")
+        pod.ssl ||= (uri.scheme == "https")
-          pod.save
+        pod.save
-        end
       end
     end
 
@@ -57,15 +60,20 @@ class Pod < ActiveRecord::Base
   end
 
   def test_connection!
-    url = "#{ssl ? 'https' : 'http'}://#{host}"
+    result = ConnectionTester.check uri.to_s
-    result = ConnectionTester.check url
+    logger.debug "tested pod: '#{uri}' - #{result.inspect}"
-    logger.info "testing pod: '#{url}' - #{result.inspect}"
 
     transaction do
       update_from_result(result)
     end
   end
 
+  # @param path [String]
+  # @return [String]
+  def url_to(path)
+    uri.tap {|uri| uri.path = path }.to_s
+  end
+
   private
 
   def update_from_result(result)
@@ -85,6 +93,7 @@ class Pod < ActiveRecord::Base
   end
 
   def attributes_from_result(result)
+    self.ssl ||= result.ssl
     self.error = result.failure_message[0..254] if result.error?
     self.software = result.software_version[0..254] if result.software_version.present?
     self.response_time = result.rt
@@ -97,4 +106,10 @@ class Pod < ActiveRecord::Base
       :no_errors
     end
   end
+
+  # @return [URI]
+  def uri
+    @uri ||= (ssl ? URI::HTTPS : URI::HTTP).build(host: host, port: port)
+    @uri.dup
+  end
 end

app/models/user.rb

@@ -39,7 +39,7 @@ class User < ActiveRecord::Base
 
   serialize :hidden_shareables, Hash
 
-  has_one :person, :foreign_key => :owner_id
+  has_one :person, inverse_of: :owner, foreign_key: :owner_id
   has_one :profile, through: :person
 
   delegate :guid, :public_key, :posts, :photos, :owns?, :image_url,
@@ -82,8 +82,7 @@ class User < ActiveRecord::Base
 
   has_many :share_visibilities
 
-  before_save :guard_unconfirmed_email,
+  before_save :guard_unconfirmed_email
-              :save_person!
 
   def self.all_sharing_with_person(person)
     User.joins(:contacts).where(:contacts => {:person_id => person.id})
@@ -460,7 +459,6 @@ class User < ActiveRecord::Base
   end
 
   def set_person(person)
-    person.url = AppConfig.pod_uri.to_s
     person.diaspora_handle = "#{self.username}#{User.diaspora_id_host}"
     self.person = person
   end
@@ -539,16 +537,6 @@ class User < ActiveRecord::Base
     end
   end
 
-  # Sometimes we access the person in a strange way and need to do this
-  # @note we should make this method depricated.
-  #
-  # @return [Person]
-  def save_person!
-    self.person.save if self.person && self.person.changed?
-    self.person
-  end
-
-
   def no_person_with_same_username
     diaspora_id = "#{self.username}#{User.diaspora_id_host}"
     if self.username_changed? && Person.exists?(:diaspora_handle => diaspora_id)

app/presenters/pod_presenter.rb

@@ -3,6 +3,7 @@ class PodPresenter < BasePresenter
     {
       id:            id,
       host:          host,
+      port:          port,
       ssl:           ssl,
       status:        status,
       checked_at:    checked_at,

config/initializers/diaspora_federation.rb

@@ -47,7 +47,7 @@ DiasporaFederation.configure do |config|
       # find existing person or create a new one
       person_entity = Person.find_by(diaspora_handle: person.diaspora_id) ||
         Person.new(diaspora_handle: person.diaspora_id, guid: person.guid,
-                   serialized_public_key: person.exported_key, url: person.url)
+                   serialized_public_key: person.exported_key, pod: Pod.find_or_create_by(url: person.url))
 
       profile = person.profile
       profile_entity = person_entity.profile ||= Profile.new

config/locales/javascript/javascript.en.yml

@@ -72,6 +72,9 @@ en:
         unchecked:
           one: "There is still one pod that hasn't been checked at all."
           other: "There are still <%= count %> pods that haven't been checked at all."
+        version_failed:
+          one: "There is one pod that has no version (old pod, no NodeInfo)."
+          other: "There are <%= count %> pods that have no version (old pods, no NodeInfo)."
         errors:
           one: "The connection test returned an error for one pod."
           other: "The connection test returned an error for <%= count %> pods."

db/migrate/20160124234712_extend_pods.rb

@@ -0,0 +1,77 @@
+class ExtendPods < ActiveRecord::Migration
+  class Pod < ActiveRecord::Base
+    has_many :people
+
+    DEFAULT_PORTS = [URI::HTTP::DEFAULT_PORT, URI::HTTPS::DEFAULT_PORT]
+
+    def self.find_or_create_by(opts)
+      uri = URI.parse(opts.fetch(:url))
+      port = DEFAULT_PORTS.include?(uri.port) ? nil : uri.port
+      find_or_initialize_by(host: uri.host, port: port).tap do |pod|
+        pod.ssl ||= (uri.scheme == "https")
+        pod.save
+      end
+    end
+
+    def url
+      (ssl ? URI::HTTPS : URI::HTTP).build(host: host, port: port, path: "/")
+    end
+  end
+
+  class Person < ActiveRecord::Base
+    belongs_to :owner, class_name: "User"
+    belongs_to :pod
+
+    def url
+      owner_id.nil? ? pod.url.to_s : AppConfig.url_to("/")
+    end
+  end
+
+  class User < ActiveRecord::Base
+    has_one :person, inverse_of: :owner, foreign_key: :owner_id
+  end
+
+  def up
+    remove_index :pods, :host
+
+    # add port
+    add_column :pods, :port, :integer
+    add_index :pods, %i(host port), unique: true, length: {host: 190, port: nil}, using: :btree
+
+    add_column :pods, :blocked, :boolean, default: false
+
+    Pod.reset_column_information
+
+    # link people with pod
+    add_column :people, :pod_id, :integer
+    add_index :people, :url, length: 190
+    add_foreign_key :people, :pods, name: :people_pod_id_fk, on_delete: :cascade
+    Person.where(owner: nil).group_by {|person| person[:url] }.each do |url, _|
+      pod = Pod.find_or_create_by(url: url)
+      Person.where(url: url).update_all(pod_id: pod.id)
+    end
+
+    # cleanup unused pods
+    Pod.joins("LEFT OUTER JOIN people ON pods.id = people.pod_id").delete_all("people.id is NULL")
+
+    remove_column :people, :url
+  end
+
+  def down
+    # restore url
+    add_column :people, :url, :text
+    Person.all.group_by(&:pod_id).each do |pod_id, persons|
+      Person.where(pod_id: pod_id).update_all(url: persons.first.url)
+    end
+    change_column :people, :url, :text, null: false
+    remove_foreign_key :people, :pods
+    remove_column :people, :pod_id
+
+    # remove pods with port
+    Pod.where.not(port: nil).delete_all
+
+    remove_index :pods, column: %i(host port)
+    remove_columns :pods, :port, :blocked
+    add_index :pods, :host, unique: true, length: 190
+  end
+end

db/schema.rb

@@ -327,7 +327,6 @@ ActiveRecord::Schema.define(version: 20160225232049) do
 
   create_table "people", force: :cascade do |t|
     t.string   "guid",                  limit: 255,                   null: false
-    t.text     "url",                   limit: 65535,                 null: false
     t.string   "diaspora_handle",       limit: 255,                   null: false
     t.text     "serialized_public_key", limit: 65535,                 null: false
     t.integer  "owner_id",              limit: 4
@@ -335,11 +334,13 @@ ActiveRecord::Schema.define(version: 20160225232049) do
     t.datetime "updated_at",                                          null: false
     t.boolean  "closed_account",                      default: false
     t.integer  "fetch_status",          limit: 4,     default: 0
+    t.integer  "pod_id",                limit: 4
   end
 
   add_index "people", ["diaspora_handle"], name: "index_people_on_diaspora_handle", unique: true, length: {"diaspora_handle"=>191}, using: :btree
   add_index "people", ["guid"], name: "index_people_on_guid", unique: true, length: {"guid"=>191}, using: :btree
   add_index "people", ["owner_id"], name: "index_people_on_owner_id", unique: true, using: :btree
+  add_index "people", ["pod_id"], name: "people_pod_id_fk", using: :btree
 
   create_table "photos", force: :cascade do |t|
     t.integer  "tmp_old_id",          limit: 4
@@ -375,10 +376,12 @@ ActiveRecord::Schema.define(version: 20160225232049) do
     t.integer  "response_time", limit: 4,   default: -1
     t.string   "software",      limit: 255
     t.string   "error",         limit: 255
+    t.integer  "port",          limit: 4
+    t.boolean  "blocked",                   default: false
   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", ["host", "port"], name: "index_pods_on_host_and_port", unique: true, length: {"host"=>190, "port"=>nil}, 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
 
@@ -672,6 +675,7 @@ ActiveRecord::Schema.define(version: 20160225232049) do
   add_foreign_key "notification_actors", "notifications", name: "notification_actors_notification_id_fk", on_delete: :cascade
   add_foreign_key "o_auth_access_tokens", "authorizations"
   add_foreign_key "o_auth_applications", "users"
+  add_foreign_key "people", "pods", name: "people_pod_id_fk", on_delete: :cascade
   add_foreign_key "posts", "people", column: "author_id", name: "posts_author_id_fk", on_delete: :cascade
   add_foreign_key "ppid", "o_auth_applications"
   add_foreign_key "ppid", "users"

lib/account_deleter.rb

@@ -104,11 +104,11 @@ class AccountDeleter
   end
 
   def normal_ar_person_associates_to_delete
-    [:posts, :photos, :mentions, :participations, :roles]
+    %i(posts photos mentions participations roles)
   end
 
   def ignored_or_special_ar_person_associations
-    [:comments, :contacts, :notification_actors, :notifications, :owner, :profile, :conversation_visibilities]
+    %i(comments contacts notification_actors notifications owner profile conversation_visibilities pod)
   end
 
   def mark_account_deletion_complete

lib/connection_tester.rb

@@ -17,11 +17,10 @@ class ConnectionTester
     #
     # @api This is the entry point you're supposed to use for testing
     #   connections to other diaspora-compatible servers.
-    # @param [String] server URL
+    # @param [String] url URL
     # @return [Result] result object containing information about the
     #   server and to what point the connection was successful
     def check(url)
-      url = "http://#{url}" unless url.include?("://")
       result = Result.new
 
       begin
@@ -54,7 +53,7 @@ class ConnectionTester
         result.reachable = false
       when SSLFailure
         result.reachable = true
-        result.ssl_status = false
+        result.ssl = false
       when HTTPFailure
         result.reachable = true
       when NodeInfoFailure
@@ -70,8 +69,6 @@ class ConnectionTester
     @uri ||= URI.parse(@url)
     raise AddressFailure,
           "invalid protocol: '#{@uri.scheme.upcase}'" unless http_uri?(@uri)
-
-    result.hostname = @uri.host
   rescue AddressFailure => e
     raise e
   rescue URI::InvalidURIError => e
@@ -83,11 +80,8 @@ class ConnectionTester
   # Perform the DNS query, the IP address will be stored in the result
   # @raise [DNSFailure] caused by a failure to resolve or a timeout
   def resolve
-    with_dns_resolver do |dns|
+    @result.ip = IPSocket.getaddress(@uri.host)
-      addr = dns.getaddress(@uri.host)
+  rescue SocketError => e
-      @result.ip = addr.to_s
-    end
-  rescue Resolv::ResolvError, Resolv::ResolvTimeout => e
     raise DNSFailure, "'#{@uri.host}' - #{e.message}"
   rescue StandardError => e
     raise Failure, e.inspect
@@ -99,13 +93,15 @@ class ConnectionTester
   # * is the SSL certificate valid (only on HTTPS)
   # * does the server return a successful HTTP status code
   # * is there a reasonable amount of redirects (3 by default)
+  # * is there a /.well-known/host-meta (this is needed to work, this can be replaced with a mandatory NodeInfo later)
   # (can't do a HEAD request, since that's not a defined route in the app)
   #
   # @raise [NetFailure, SSLFailure, HTTPFailure] if any of the checks fail
   # @return [Integer] HTTP status code
   def request
     with_http_connection do |http|
-      response = capture_response_time { http.get("/") }
+      capture_response_time { http.get("/") }
+      response = http.get("/.well-known/host-meta")
       handle_http_response(response)
     end
   rescue HTTPFailure => e
@@ -132,7 +128,7 @@ class ConnectionTester
       find_software_version(nd_resp.body)
     end
   rescue Faraday::ResourceNotFound, JSON::JSONError => e
-    raise NodeInfoFailure, e.message[0..255]
+    raise NodeInfoFailure, e.message[0..255].encode(Encoding.default_external, undef: :replace)
   rescue StandardError => e
     raise Failure, e.inspect
   end
@@ -153,39 +149,29 @@ class ConnectionTester
     yield(@http) if block_given?
   end
 
-  def with_dns_resolver
-    dns = Resolv::DNS.new
-    yield(dns) if block_given?
-  ensure
-    dns.close
-  end
-
   def http_uri?(uri)
     uri.is_a?(URI::HTTP) || uri.is_a?(URI::HTTPS)
   end
 
-  def uses_ssl?
-    @uses_ssl
-  end
-
   # request root path, measure response time
   # measured time may be skewed, if there are redirects
   #
   # @return [Faraday::Response]
   def capture_response_time
-    start     = Time.zone.now
+    start = Time.zone.now
     resp = yield if block_given?
     @result.rt = ((Time.zone.now - start) * 1000.0).to_i # milliseconds
     resp
   end
 
   def handle_http_response(response)
-    @uses_ssl = (response.env.url.scheme == "https")
     @result.status_code = Integer(response.status)
 
     if response.success?
-      @result.reachable  = true
+      raise HTTPFailure, "redirected to other hostname: #{response.env.url}" unless @uri.host == response.env.url.host
-      @result.ssl_status = @uses_ssl
+
+      @result.reachable = true
+      @result.ssl = (response.env.url.scheme == "https")
     else
       raise HTTPFailure, "unsuccessful response code: #{response.status}"
     end
@@ -228,19 +214,16 @@ class ConnectionTester
   end
 
   Result = Struct.new(
-    :hostname, :ip, :reachable, :ssl_status, :status_code, :rt, :software_version, :error
+    :ip, :reachable, :ssl, :status_code, :rt, :software_version, :error
   ) do
-    # @!attribute hostname
-    #   @return [String] hostname derived from the URL
-
     # @!attribute ip
     #   @return [String] resolved IP address from DNS query
 
     # @!attribute reachable
     #   @return [Boolean] whether the host was reachable over the network
 
-    # @!attribute ssl_status
+    # @!attribute ssl
-    #   @return [Boolean] indicating how the SSL verification went
+    #   @return [Boolean] whether the host has working ssl
 
     # @!attribute status_code
     #   @return [Integer] HTTP status code that was returned for the HEAD request

lib/hydra_wrapper.rb

@@ -86,10 +86,6 @@ class HydraWrapper
       # Save the reference to the pod to the database if not already present
       Pod.find_or_create_by(url: response.effective_url)
 
-      if redirecting_to_https? response
-        Person.url_batch_update people_for_receive_url, response.headers_hash['Location']
-      end
-
       unless response.success?
         logger.warn "event=http_multi_fail sender_id=#{@user.id} url=#{response.effective_url} " \
                     "return_code=#{response.return_code} response_code=#{response.response_code}"
@@ -101,10 +97,4 @@ class HydraWrapper
       end
     end
   end
-
-  # @return [Boolean]
-  def redirecting_to_https? response
-    response.code >= 300 && response.code < 400 &&
-    response.headers_hash['Location'] == response.request.url.sub('http://', 'https://')
-  end
 end

spec/controllers/admin/pods_controller_spec.rb

@@ -25,10 +25,11 @@ describe Admin::PodsController, type: :controller do
     end
 
     it "returns the json data" do
-      @pods = (0..2).map { FactoryGirl.create(:pod).reload } # normalize timestamps
+      3.times { FactoryGirl.create(:pod) }
+
       get :index, format: :json
 
-      expect(response.body).to eql(PodPresenter.as_collection(@pods).to_json)
+      expect(response.body).to eql(PodPresenter.as_collection(Pod.all).to_json)
     end
   end
 

spec/factories.rb

@@ -31,7 +31,7 @@ FactoryGirl.define do
 
   factory(:person, aliases: %i(author)) do
     sequence(:diaspora_handle) {|n| "bob-person-#{n}#{r_str}@example.net" }
-    url AppConfig.pod_uri.to_s
+    pod { Pod.find_or_create_by(url: "http://example.net") }
     serialized_public_key OpenSSL::PKey::RSA.generate(1024).public_key.export
     after(:build) do |person|
       unless person.profile.first_name.present?
@@ -69,10 +69,11 @@ FactoryGirl.define do
     password_confirmation { |u| u.password }
     serialized_private_key  OpenSSL::PKey::RSA.generate(1024).export
     after(:build) do |u|
-      u.person = FactoryGirl.build(:person, :profile => FactoryGirl.build(:profile),
+      u.person = FactoryGirl.build(:person,
-                                  :owner_id => u.id,
+                                   profile:               FactoryGirl.build(:profile),
-                                  :serialized_public_key => u.encryption_key.public_key.export,
+                                   pod:                   nil,
-                                  :diaspora_handle => "#{u.username}#{User.diaspora_id_host}")
+                                   serialized_public_key: u.encryption_key.public_key.export,
+                                   diaspora_handle:       "#{u.username}#{User.diaspora_id_host}")
     end
     after(:create) do |u|
       u.person.save

spec/integration/federation/federation_helper.rb

@@ -8,10 +8,13 @@ end
 
 def create_remote_user(pod)
   FactoryGirl.build(:user).tap do |user|
-    user.person = FactoryGirl.create(:person,
+    allow(user).to receive(:person).and_return(
-                                     profile:               FactoryGirl.build(:profile),
+      FactoryGirl.create(:person,
-                                     serialized_public_key: user.encryption_key.public_key.export,
+                         profile:               FactoryGirl.build(:profile),
-                                     diaspora_handle:       "#{user.username}@#{pod}")
+                         serialized_public_key: user.encryption_key.public_key.export,
+                         pod:                   Pod.find_or_create_by(url: "http://#{pod}"),
+                         diaspora_handle:       "#{user.username}@#{pod}")
+    )
     allow(DiasporaFederation.callbacks).to receive(:trigger)
                                              .with(:fetch_private_key_by_diaspora_id, user.diaspora_handle) {
                                              user.encryption_key

spec/lib/connection_tester_spec.rb

@@ -2,6 +2,10 @@
 require "spec_helper"
 
 describe ConnectionTester do
+  let(:url) { "https://pod.example.com" }
+  let(:result) { ConnectionTester::Result.new }
+  let(:tester) { ConnectionTester.new(url, result) }
+
   describe "::check" do
     it "takes a http url and returns a result object" do
       res = ConnectionTester.check("https://pod.example.com")
@@ -29,102 +33,108 @@ describe ConnectionTester do
   end
 
   describe "#resolve" do
-    before do
-      @result = ConnectionTester::Result.new
-      @dns = instance_double("Resolv::DNS")
-      allow(@dns).to receive(:close).once
-    end
-
     it "resolves the IP address" do
-      tester = ConnectionTester.new("https://pod.example.com", @result)
+      expect(IPSocket).to receive(:getaddress).with("pod.example.com").and_return("192.168.1.2")
-      expect(tester).to receive(:with_dns_resolver).and_yield(@dns)
-      expect(@dns).to receive(:getaddress).and_return("192.168.1.2")
 
       tester.resolve
-      expect(@result.ip).to eq("192.168.1.2")
+      expect(result.ip).to eq("192.168.1.2")
+    end
+
+    it "raises DNSFailure if host is unknown" do
+      expect(IPSocket).to receive(:getaddress).with("pod.example.com").and_raise(SocketError.new("Error!"))
+
+      expect { tester.resolve }.to raise_error(ConnectionTester::DNSFailure, "'pod.example.com' - Error!")
     end
   end
 
   describe "#request" do
-    before do
+    it "performs a successful GET request on '/' and '/.well-known/host-meta'" do
-      @url = "https://pod.example.com"
+      stub_request(:get, url).to_return(status: 200, body: "Hello World!")
-      @stub =
+      stub_request(:get, "#{url}/.well-known/host-meta").to_return(status: 200, body: "host-meta")
-      @result = ConnectionTester::Result.new
-      @tester = ConnectionTester.new(@url, @result)
-    end
 
-    it "performs a successful GET request on '/'" do
+      tester.request
-      stub_request(:get, @url).to_return(status: 200, body: "Hello World!")
+      expect(result.rt).to be > -1
-
+      expect(result.reachable).to be_truthy
-      @tester.request
+      expect(result.ssl).to be_truthy
-      expect(@result.rt).to be > -1
-      expect(@result.reachable).to be_truthy
-      expect(@result.ssl_status).to be_truthy
     end
 
     it "receives a 'normal' 301 redirect" do
-      stub_request(:get, @url).to_return(status: 301, headers: {"Location" => "#{@url}/redirect"})
+      stub_request(:get, url).to_return(status: 301, headers: {"Location" => "#{url}/redirect"})
-      stub_request(:get, "#{@url}/redirect").to_return(status: 200, body: "Hello World!")
+      stub_request(:get, "#{url}/redirect").to_return(status: 200, body: "Hello World!")
+      stub_request(:get, "#{url}/.well-known/host-meta").to_return(status: 200, body: "host-meta")
 
-      @tester.request
+      tester.request
+    end
+
+    it "updates ssl after https redirect" do
+      tester = ConnectionTester.new("http://pod.example.com/", result)
+      stub_request(:get, "http://pod.example.com/").to_return(status: 200, body: "Hello World!")
+      stub_request(:get, "http://pod.example.com/.well-known/host-meta")
+        .to_return(status: 301, headers: {"Location" => "#{url}/.well-known/host-meta"})
+      stub_request(:get, "#{url}/.well-known/host-meta").to_return(status: 200, body: "host-meta")
+
+      tester.request
+      expect(result.ssl).to be_truthy
+    end
+
+    it "rejects other hostname after redirect redirect" do
+      stub_request(:get, url).to_return(status: 200, body: "Hello World!")
+      stub_request(:get, "#{url}/.well-known/host-meta")
+        .to_return(status: 301, headers: {"Location" => "https://example.com/.well-known/host-meta"})
+      stub_request(:get, "https://example.com/.well-known/host-meta").to_return(status: 200, body: "host-meta")
+
+      expect { tester.request }.to raise_error(ConnectionTester::HTTPFailure)
     end
 
     it "receives too many 301 redirects" do
-      stub_request(:get, @url).to_return(status: 301, headers: {"Location" => "#{@url}/redirect"})
+      stub_request(:get, url).to_return(status: 301, headers: {"Location" => "#{url}/redirect"})
-      stub_request(:get, "#{@url}/redirect").to_return(status: 301, headers: {"Location" => "#{@url}/redirect1"})
+      stub_request(:get, "#{url}/redirect").to_return(status: 301, headers: {"Location" => "#{url}/redirect1"})
-      stub_request(:get, "#{@url}/redirect1").to_return(status: 301, headers: {"Location" => "#{@url}/redirect2"})
+      stub_request(:get, "#{url}/redirect1").to_return(status: 301, headers: {"Location" => "#{url}/redirect2"})
-      stub_request(:get, "#{@url}/redirect2").to_return(status: 301, headers: {"Location" => "#{@url}/redirect3"})
+      stub_request(:get, "#{url}/redirect2").to_return(status: 301, headers: {"Location" => "#{url}/redirect3"})
-      stub_request(:get, "#{@url}/redirect3").to_return(status: 200, body: "Hello World!")
+      stub_request(:get, "#{url}/redirect3").to_return(status: 200, body: "Hello World!")
 
-      expect { @tester.request }.to raise_error(ConnectionTester::HTTPFailure)
+      expect { tester.request }.to raise_error(ConnectionTester::HTTPFailure)
     end
 
     it "receives a 404 not found" do
-      stub_request(:get, @url).to_return(status: 404, body: "Not Found!")
+      stub_request(:get, url).to_return(status: 404, body: "Not Found!")
-      expect { @tester.request }.to raise_error(ConnectionTester::HTTPFailure)
+      expect { tester.request }.to raise_error(ConnectionTester::HTTPFailure)
     end
 
     it "cannot connect" do
-      stub_request(:get, @url).to_raise(Faraday::ConnectionFailed.new("Error!"))
+      stub_request(:get, url).to_raise(Faraday::ConnectionFailed.new("Error!"))
-      expect { @tester.request }.to raise_error(ConnectionTester::NetFailure)
+      expect { tester.request }.to raise_error(ConnectionTester::NetFailure)
     end
 
     it "encounters an invalid SSL setup" do
-      stub_request(:get, @url).to_raise(Faraday::SSLError.new("Error!"))
+      stub_request(:get, url).to_raise(Faraday::SSLError.new("Error!"))
-      expect { @tester.request }.to raise_error(ConnectionTester::SSLFailure)
+      expect { tester.request }.to raise_error(ConnectionTester::SSLFailure)
     end
   end
 
   describe "#nodeinfo" do
-    before do
-      @url = "https://diaspora.example.com"
-      @result = ConnectionTester::Result.new
-      @tester = ConnectionTester.new(@url, @result)
-
-      @ni_wellknown = {links: [{rel:  ConnectionTester::NODEINFO_SCHEMA,
-                                href: "/nodeinfo"}]}
-      @ni_document = {software: {name: "diaspora", version: "a.b.c.d"}}
-    end
-
     it "reads the version from the nodeinfo document" do
-      stub_request(:get, "#{@url}#{ConnectionTester::NODEINFO_FRAGMENT}")
+      ni_wellknown = {links: [{rel: ConnectionTester::NODEINFO_SCHEMA, href: "/nodeinfo"}]}
-        .to_return(status: 200, body: JSON.generate(@ni_wellknown))
+      ni_document = {software: {name: "diaspora", version: "a.b.c.d"}}
-      stub_request(:get, "#{@url}/nodeinfo").to_return(status: 200, body: JSON.generate(@ni_document))
 
-      @tester.nodeinfo
+      stub_request(:get, "#{url}#{ConnectionTester::NODEINFO_FRAGMENT}")
-      expect(@result.software_version).to eq("diaspora a.b.c.d")
+        .to_return(status: 200, body: JSON.generate(ni_wellknown))
+      stub_request(:get, "#{url}/nodeinfo").to_return(status: 200, body: JSON.generate(ni_document))
+
+      tester.nodeinfo
+      expect(result.software_version).to eq("diaspora a.b.c.d")
     end
 
     it "handles a missing nodeinfo document gracefully" do
-      stub_request(:get, "#{@url}#{ConnectionTester::NODEINFO_FRAGMENT}")
+      stub_request(:get, "#{url}#{ConnectionTester::NODEINFO_FRAGMENT}")
         .to_return(status: 404, body: "Not Found")
-      expect { @tester.nodeinfo }.to raise_error(ConnectionTester::NodeInfoFailure)
+      expect { tester.nodeinfo }.to raise_error(ConnectionTester::NodeInfoFailure)
     end
 
     it "handles a malformed document gracefully" do
-      stub_request(:get, "#{@url}#{ConnectionTester::NODEINFO_FRAGMENT}")
+      stub_request(:get, "#{url}#{ConnectionTester::NODEINFO_FRAGMENT}")
         .to_return(status: 200, body: '{"json"::::"malformed"}')
-      expect { @tester.nodeinfo }.to raise_error(ConnectionTester::NodeInfoFailure)
+      expect { tester.nodeinfo }.to raise_error(ConnectionTester::NodeInfoFailure)
     end
   end
 end

spec/lib/diaspora/fetcher/public_spec.rb

@@ -13,9 +13,9 @@ describe Diaspora::Fetcher::Public do
     # the guid of the person is "7445f9a0a6c28ebb"
     @fixture = File.open(Rails.root.join('spec', 'fixtures', 'public_posts.json')).read
     @fetcher = Diaspora::Fetcher::Public.new
-    @person = FactoryGirl.create(:person, {:guid => "7445f9a0a6c28ebb",
+    @person = FactoryGirl.create(:person, guid:            "7445f9a0a6c28ebb",
-                                :url => "https://remote-testpod.net",
+                                          pod:             Pod.find_or_create_by(url: "https://remote-testpod.net"),
-                                :diaspora_handle => "testuser@remote-testpod.net"})
+                                          diaspora_handle: "testuser@remote-testpod.net")
 
     stub_request(:get, /remote-testpod.net\/people\/.*\/stream/)
       .with(headers: {

spec/lib/hydra_wrapper_spec.rb

@@ -74,37 +74,4 @@ describe HydraWrapper do
     end
 
   end
-
-  describe '#redirecting_to_https?!' do
-    it 'does not execute unless response has a 3xx code' do
-      resp = double code: 200
-      expect(@wrapper.send(:redirecting_to_https?, resp)).to be false
-    end
-
-    it "returns true if just the protocol is different" do
-      host = "the-same.com/"
-      resp = double(
-        request: double(url: "http://#{host}"),
-        code: 302,
-        headers_hash: {
-          'Location' => "https://#{host}"
-        }
-      )
-
-      expect(@wrapper.send(:redirecting_to_https?, resp)).to be true
-    end
-
-    it "returns false if not just the protocol is different" do
-      host = "the-same.com/"
-      resp = double(
-        request: double(url: "http://#{host}"),
-        code: 302,
-        headers_hash: {
-          'Location' => "https://not-the-same/"
-        }
-      )
-
-      expect(@wrapper.send(:redirecting_to_https?, resp)).to be false
-    end
-  end
 end

spec/lib/postzord/receiver/public_spec.rb

@@ -19,6 +19,10 @@ describe Postzord::Receiver::Public do
       comment.save
       #bob signs his comment, and then sends it up
       xml = Salmon::Slap.create_by_user_and_activity(bob, comment.to_diaspora_xml).xml_for(nil)
+      person = bob.person
+      person.owner = nil
+      person.pod = Pod.find_or_create_by(url: AppConfig.pod_uri)
+      person.save
       bob.destroy
       comment.destroy
       expect{

spec/models/person_spec.rb

@@ -109,7 +109,7 @@ describe Person, :type => :model do
 
   describe "valid url" do
     context "https urls" do
-      let(:person) { FactoryGirl.build(:person, url: "https://example.com") }
+      let(:person) { FactoryGirl.build(:person, pod: Pod.find_or_create_by(url: "https://example.com")) }
 
       it "should add trailing slash" do
         expect(person.url).to eq("https://example.com/")
@@ -129,7 +129,9 @@ describe Person, :type => :model do
     end
 
     context "messed up urls" do
-      let(:person) { FactoryGirl.build(:person, url: "https://example.com/a/bit/messed/up") }
+      let(:person) {
+        FactoryGirl.build(:person, pod: Pod.find_or_create_by(url: "https://example.com/a/bit/messed/up"))
+      }
 
       it "should return the correct url" do
         expect(person.url).to eq("https://example.com/")
@@ -149,12 +151,12 @@ describe Person, :type => :model do
     end
 
     it "should allow ports in the url" do
-      person = FactoryGirl.build(:person, url: "https://example.com:3000/")
+      person = FactoryGirl.build(:person, pod: Pod.find_or_create_by(url: "https://example.com:3000/"))
       expect(person.url).to eq("https://example.com:3000/")
     end
 
     it "should remove https port in the url" do
-      person = FactoryGirl.build(:person, url: "https://example.com:443/")
+      person = FactoryGirl.build(:person, pod: Pod.find_or_create_by(url: "https://example.com:443/"))
       expect(person.url).to eq("https://example.com/")
     end
   end
@@ -538,32 +540,6 @@ describe Person, :type => :model do
     end
   end
 
-  context 'updating urls' do
-    before do
-      @url = "http://new-url.com/"
-    end
-
-    describe '.url_batch_update' do
-      it "calls #update_person_url given an array of users and a url" do
-        people = [double.as_null_object, double.as_null_object, double.as_null_object]
-        people.each do |person|
-          expect(person).to receive(:update_url).with(@url)
-        end
-        Person.url_batch_update(people, @url)
-      end
-    end
-
-    describe '#update_url' do
-      it "updates a given person's url" do
-        expect {
-          alice.person.update_url(@url)
-        }.to change {
-          alice.person.reload.url
-        }.from(anything).to(@url)
-      end
-    end
-  end
-
   describe '#lock_access!' do
     it 'sets the closed_account flag' do
       @person.lock_access!

spec/models/pod_spec.rb

@@ -1,19 +1,45 @@
 require "spec_helper"
 
 describe Pod, type: :model do
-  describe "::find_or_create_by" do
+  describe ".find_or_create_by" do
     it "takes a url, and makes one by host" do
-      pod = Pod.find_or_create_by(url: "https://joindiaspora.com/maxwell")
+      pod = Pod.find_or_create_by(url: "https://example.org/u/maxwell")
-      expect(pod.host).to eq("joindiaspora.com")
+      expect(pod.host).to eq("example.org")
     end
 
-    it "sets ssl boolean (side-effect)" do
+    it "saves the port" do
-      pod = Pod.find_or_create_by(url: "https://joindiaspora.com/maxwell")
+      pod = Pod.find_or_create_by(url: "https://example.org:3000/")
+      expect(pod.host).to eq("example.org")
+      expect(pod.port).to eq(3000)
+    end
+
+    it "ignores default ports" do
+      pod = Pod.find_or_create_by(url: "https://example.org:443/")
+      expect(pod.host).to eq("example.org")
+      expect(pod.port).to be_nil
+    end
+
+    it "sets ssl boolean" do
+      pod = Pod.find_or_create_by(url: "https://example.org/")
+      expect(pod.ssl).to be true
+    end
+
+    it "updates ssl boolean if upgraded to https" do
+      pod = Pod.find_or_create_by(url: "http://joindiaspora.com/")
+      expect(pod.ssl).to be false
+      pod = Pod.find_or_create_by(url: "https://joindiaspora.com/")
+      expect(pod.ssl).to be true
+    end
+
+    it "does not update ssl boolean if downgraded to http" do
+      pod = Pod.find_or_create_by(url: "https://joindiaspora.com/")
+      expect(pod.ssl).to be true
+      pod = Pod.find_or_create_by(url: "http://joindiaspora.com/")
       expect(pod.ssl).to be true
     end
   end
 
-  describe "::check_all!" do
+  describe ".check_all!" do
     before do
       @pods = (0..4).map do
         double("pod").tap do |pod|
@@ -75,4 +101,11 @@ describe Pod, type: :model do
       end
     end
   end
+
+  describe "#url_to" do
+    it "appends the path to the pod-url" do
+      pod = FactoryGirl.create(:pod)
+      expect(pod.url_to("/receive/public")).to eq("https://#{pod.host}/receive/public")
+    end
+  end
 end

spec/models/user/querying_spec.rb

@@ -200,7 +200,9 @@ describe User::Querying, :type => :model do
         connect_users(alice, @alices_aspect, remote_user, asp3)
 
         local_person = remote_user.person
-        local_person.owner_id = nil
+        local_person.diaspora_handle = "#{remote_user.username}@example.net"
+        local_person.owner = nil
+        local_person.pod = Pod.find_or_create_by(url: "http://example.net")
         local_person.save
         local_person.reload
 

spec/models/user_spec.rb

@@ -111,21 +111,6 @@ describe User, :type => :model do
     end
   end
 
-  context 'callbacks' do
-    describe '#save_person!' do
-      it 'saves the corresponding user if it has changed' do
-        alice.person.url = "http://stuff.com"
-        expect_any_instance_of(Person).to receive(:save)
-        alice.save
-      end
-
-      it 'does not save the corresponding user if it has not changed' do
-        expect_any_instance_of(Person).not_to receive(:save)
-        alice.save
-      end
-    end
-  end
-
   describe 'hidden_shareables' do
     before do
       @sm = FactoryGirl.create(:status_message)
@@ -1192,8 +1177,7 @@ describe User, :type => :model do
       invited_user.save(validate: false)
 
       closed_account = FactoryGirl.create(:user)
-      closed_account.person.closed_account = true
+      closed_account.person.lock_access!
-      closed_account.save
     end
 
     it "returns total_users excluding closed accounts & users without usernames" do

spec/workers/http_multi_spec.rb

@@ -108,21 +108,20 @@ describe Workers::HttpMulti do
     Workers::HttpMulti.new.perform bob.id, @post_xml, [person.id], "Postzord::Dispatcher::Private"
   end
 
-  it 'updates http users who have moved to https' do
+  it "updates http users who have moved to https" do
     person = @people.first
-    person.update_url("http://remote.net/")
 
     response = Typhoeus::Response.new(
-      code: 301,
+      code:             301,
-      effective_url: 'https://foobar.com',
+      effective_url:    "https://example.net",
       response_headers: "Location: #{person.receive_url.sub('http://', 'https://')}",
-      body: "",
+      body:             "",
-      time: 0.2
+      time:             0.2
     )
     Typhoeus.stub(person.receive_url).and_return response
 
     Workers::HttpMulti.new.perform bob.id, @post_xml, [person.id], "Postzord::Dispatcher::Private"
-    expect(Person.find(person.id).url).to eq("https://remote.net/")
+    expect(Person.find(person.id).url).to eq("https://example.net/")
   end
 
   it 'only sends to users with valid RSA keys' do