Merge branch 'stable' into develop

Changelog.md

@@ -51,6 +51,7 @@ bind to an UNIX socket at `unix:tmp/diaspora.sock`. Please change your local
 * Remove top margin for first heading in a post [#6110](https://github.com/diaspora/diaspora/pull/6110)
 * Add link to pod statistics in right navigation [#6117](https://github.com/diaspora/diaspora/pull/6117)
 * Update to Rails 4.2.3 [#6140](https://github.com/diaspora/diaspora/pull/6140)
+* Refactor person related URL generation [#6168](https://github.com/diaspora/diaspora/pull/6168)
 
 ## Bug fixes
 * Precompile facebox images [#6105](https://github.com/diaspora/diaspora/pull/6105)

app/helpers/layout_helper.rb

@@ -32,7 +32,8 @@ module LayoutHelper
 
   def current_user_atom_tag
     return unless @person.present?
-    content_tag(:link, '', :rel => 'alternate', :href => "#{@person.public_url}.atom", :type => "application/atom+xml", :title => t('.public_feed', :name => @person.name))
+    content_tag(:link, "", rel: "alternate", href: @person.atom_url, type: "application/atom+xml",
+                title: t(".public_feed", name: @person.name))
   end
 
   def translation_missing_warnings

app/helpers/publics_helper.rb

-#   Copyright (c) 2010-2011, Diaspora Inc.  This file is
-#   licensed under the Affero General Public License version 3 or later.  See
-#   the COPYRIGHT file.
-
-module PublicsHelper
-  def subscribe(opts = {})
-    subscriber = Subscriber.first(:url => opts[:callback], :topic => opts[:topic])
-    subscriber ||= Subscriber.new(:url => opts[:callback], :topic => opts[:topic])
-
-    if subscriber.save
-      if opts[:verify] == 'sync'
-        204
-      elsif opts[:verify] == 'async'
-        202
-      end
-    else
-      400
-    end
-  end
-
-end

app/models/person.rb

@@ -195,33 +195,30 @@ class Person < ActiveRecord::Base
               end
   end
 
+  def username
+    @username ||= owner ? owner.username : diaspora_handle.split("@")[0]
+  end
+
   def owns?(obj)
     self.id == obj.author_id
   end
 
   def url
-    begin
-      uri = URI.parse(self[:url])
-      url = "#{uri.scheme}://#{uri.host}"
-      url += ":#{uri.port}" unless ["80", "443"].include?(uri.port.to_s)
-      url += "/"
-    rescue => e
-      url = self[:url]
-    end
-    url
+    url_to "/"
+  rescue
+    self[:url]
   end
 
-  def receive_url
-    "#{url}receive/users/#{self.guid}/"
+  def profile_url
+    url_to "/u/#{username}"
   end
 
-  def public_url
-    if self.owner
-      username = self.owner.username
-    else
-      username = self.diaspora_handle.split("@")[0]
-    end
-    "#{url}public/#{username}"
+  def atom_url
+    url_to "/public/#{username}.atom"
+  end
+
+  def receive_url
+    url_to "/receive/users/#{guid}"
   end
 
   def public_key_hash
@@ -263,7 +260,6 @@ class Person < ActiveRecord::Base
     #hcard_profile = HCard.find profile.hcard.first[:href]
     ::Logging::Logger[self].info "event=webfinger_marshal valid=#{new_person.valid?} " \
                                  "target=#{new_person.diaspora_handle}"
-    new_person.url = hcard[:url]
     new_person.assign_new_profile_from_hcard(hcard)
     new_person.save!
     new_person.profile.save!
@@ -321,11 +317,9 @@ class Person < ActiveRecord::Base
   # @param person [Person]
   # @param url [String]
   def update_url(url)
-    location = URI.parse(url)
-    newuri = "#{location.scheme}://#{location.host}"
-    newuri += ":#{location.port}" unless ["80", "443"].include?(location.port.to_s)
-    newuri += "/"
-    self.update_attributes(:url => newuri)
+    @uri = URI.parse(url)
+    @uri.path = "/"
+    update_attributes(:url => @uri.to_s)
   end
 
   def lock_access!
@@ -349,6 +343,18 @@ class Person < ActiveRecord::Base
 
   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
+  end
+
   def fix_profile
     Webfinger.new(self.diaspora_handle).fetch
     self.reload

app/models/user.rb

@@ -43,7 +43,7 @@ class User < ActiveRecord::Base
   has_one :profile, through: :person
 
   delegate :guid, :public_key, :posts, :photos, :owns?, :image_url,
-           :diaspora_handle, :name, :public_url, :profile, :url,
+           :diaspora_handle, :name, :atom_url, :profile_url, :profile, :url,
            :first_name, :last_name, :gender, :participations, to: :person
   delegate :id, :guid, to: :person, prefix: true
 

app/views/publics/webfinger.erb

@@ -7,7 +7,7 @@
   <Link rel="http://joindiaspora.com/guid" type = 'text/html' href="<%=@person.guid%>"/>
 
   <Link rel='http://webfinger.net/rel/profile-page' type='text/html' <%=person_href(@person, :absolute => true)%>/>
-  <Link rel="http://schemas.google.com/g/2010#updates-from" type="application/atom+xml" href="<%=@person.public_url%>.atom"/>
+  <Link rel="http://schemas.google.com/g/2010#updates-from" type="application/atom+xml" href="<%=@person.atom_url%>"/>
   <Link rel="salmon" href="<%= @person.receive_url %>"/>
 
   <Link rel="diaspora-public-key" type = 'RSA' href="<%=Base64.strict_encode64(@person.exported_key)%>"/>

app/views/shared/_public_explain.haml

@@ -16,7 +16,7 @@
           = t(".title")
       .modal-body
         %p=t('.outside')
-        %p= link_to t('.atom_feed'), "#{current_user.public_url}.atom"
+        %p= link_to t(".atom_feed"), current_user.atom_url
         - if current_user.services
           - for service in current_user.services
             %p= t('.logged_in', :service => service.provider)

app/views/users/public.atom.builder

-atom_feed({'xmlns:thr' => 'http://purl.org/syndication/thread/1.0',
- 'xmlns:georss' => 'http://www.georss.org/georss',
- 'xmlns:activity' => 'http://activitystrea.ms/spec/1.0/',
- 'xmlns:media' => 'http://purl.org/syndication/atommedia',
- 'xmlns:poco' => 'http://portablecontacts.net/spec/1.0',
- 'xmlns:ostatus' => 'http://ostatus.org/schema/1.0',
- 'xmlns:statusnet' => 'http://status.net/schema/api/1/',
- :id => "#{@user.public_url}.atom",
- :root_url => "#{@user.public_url}"}) do |feed|
+atom_feed("xmlns:thr"       => "http://purl.org/syndication/thread/1.0",
+          "xmlns:georss"    => "http://www.georss.org/georss",
+          "xmlns:activity"  => "http://activitystrea.ms/spec/1.0/",
+          "xmlns:media"     => "http://purl.org/syndication/atommedia",
+          "xmlns:poco"      => "http://portablecontacts.net/spec/1.0",
+          "xmlns:ostatus"   => "http://ostatus.org/schema/1.0",
+          "xmlns:statusnet" => "http://status.net/schema/api/1/",
+          :id               => @user.atom_url,
+          :root_url         => @user.profile_url) do |feed|
 
   feed.tag! :generator, 'Diaspora', :uri => "#{AppConfig.pod_uri.to_s}"
   feed.title "#{@user.name}'s Public Feed"

app/workers/publish_to_hub.rb

@@ -6,9 +6,8 @@ module Workers
   class PublishToHub < Base
     sidekiq_options queue: :http_service
 
-    def perform(sender_public_url)
-      atom_url = sender_public_url + '.atom'
-      Pubsubhubbub.new(AppConfig.environment.pubsub_server.get).publish(atom_url)
+    def perform(sender_atom_url)
+      Pubsubhubbub.new(AppConfig.environment.pubsub_server.get).publish(sender_atom_url)
     end
   end
 end

features/support/poor_mans_webmock.rb

@@ -4,25 +4,19 @@
 
 module Workers
   class PublishToHub < Base
-    def perform(sender_public_url)
+    def perform(_sender_atom_url)
       # don't publish to pubsubhubbub in cucumber
     end
   end
 
   class HttpMulti < Base
-    def perform(user_id, enc_object_xml, person_ids, retry_count=0)
+    def perform(_user_id, _enc_object_xml, _person_ids, _retry_count=0)
       # don't federate in cucumber
     end
   end
 
-  class HttpPost < Base
-    def perform(url, body, tries_remaining = NUM_TRIES)
-      # don't post to outside services in cucumber
-    end
-  end
-
   class PostToService < Base
-    def perform(service_id, post_id, url)
+    def perform(_service_id, _post_id, _url)
       # don't post to services in cucumber
     end
   end

lib/postzord/dispatcher.rb

@@ -143,7 +143,7 @@ class Postzord::Dispatcher
 
   def deliver_to_hub
     logger.debug "event=post_to_service type=pubsub sender_handle=#{@sender.diaspora_handle}"
-    Workers::PublishToHub.perform_async(@sender.public_url)
+    Workers::PublishToHub.perform_async(@sender.atom_url)
   end
 
   # @param url [String]

spec/lib/postzord/dispatcher_spec.rb

@@ -278,7 +278,7 @@ describe Postzord::Dispatcher do
 
       it 'queues a job to notify the hub' do
         allow(Workers::PostToService).to receive(:perform_async).with(anything, anything, anything)
-        expect(Workers::PublishToHub).to receive(:perform_async).with(alice.public_url)
+        expect(Workers::PublishToHub).to receive(:perform_async).with(alice.atom_url)
         @zord.send(:deliver_to_services, nil, [])
       end
 

spec/models/person_spec.rb

@@ -108,20 +108,55 @@ describe Person, :type => :model do
   end
 
   describe "valid url" do
-    it 'should allow for https urls' do
-      person = FactoryGirl.build(:person, :url => "https://example.com")
-      expect(person).to be_valid
+    context "https urls" do
+      let(:person) { FactoryGirl.build(:person, url: "https://example.com") }
+
+      it "should add trailing slash" do
+        expect(person.url).to eq("https://example.com/")
+      end
+
+      it "should return the receive url" do
+        expect(person.receive_url).to eq("https://example.com/receive/users/#{person.guid}")
+      end
+
+      it "should return the atom url" do
+        expect(person.atom_url).to eq("https://example.com/public/#{person.username}.atom")
+      end
+
+      it "should return the profile url" do
+        expect(person.profile_url).to eq("https://example.com/u/#{person.username}")
+      end
     end
 
-    it 'should always return the correct receive url' do
-      person = FactoryGirl.build(:person, :url => "https://example.com/a/bit/messed/up")
-      expect(person.receive_url).to eq("https://example.com/receive/users/#{person.guid}/")
+    context "messed up urls" do
+      let(:person) { FactoryGirl.build(:person, url: "https://example.com/a/bit/messed/up") }
+
+      it "should return the correct url" do
+        expect(person.url).to eq("https://example.com/")
+      end
+
+      it "should return the correct receive url" do
+        expect(person.receive_url).to eq("https://example.com/receive/users/#{person.guid}")
+      end
+
+      it "should return the correct atom url" do
+        expect(person.atom_url).to eq("https://example.com/public/#{person.username}.atom")
+      end
+
+      it "should return the correct profile url" do
+        expect(person.profile_url).to eq("https://example.com/u/#{person.username}")
+      end
     end
 
-    it 'should allow ports in the url' do
-      person = FactoryGirl.build(:person, :url => "https://example.com:3000/")
+    it "should allow ports in the url" do
+      person = FactoryGirl.build(:person, 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/")
+      expect(person.url).to eq("https://example.com/")
+    end
   end
 
   describe '#diaspora_handle' do

spec/workers/http_multi_spec.rb

@@ -110,8 +110,7 @@ describe Workers::HttpMulti do
 
   it 'updates http users who have moved to https' do
     person = @people.first
-    person.url = 'http://remote.net/'
-    person.save
+    person.update_url("http://remote.net/")
 
     response = Typhoeus::Response.new(
       code: 301,
@@ -123,8 +122,7 @@ describe Workers::HttpMulti do
     Typhoeus.stub(person.receive_url).and_return response
 
     Workers::HttpMulti.new.perform bob.id, @post_xml, [person.id], "Postzord::Dispatcher::Private"
-    person.reload
-    expect(person.url).to eq("https://remote.net/")
+    expect(Person.find(person.id).url).to eq("https://remote.net/")
   end
 
   it 'only sends to users with valid RSA keys' do

spec/workers/publish_to_hub_spec.rb

@@ -2,15 +2,15 @@
 #   licensed under the Affero General Public License version 3 or later.  See
 #   the COPYRIGHT file.
 
-require 'spec_helper'
+require "spec_helper"
 
 describe Workers::PublishToHub do
-  describe '.perform' do
-    it 'calls pubsubhubbub' do
-      url = "http://publiczone.com/"
-      m = double()
+  describe ".perform" do
+    it "calls pubsubhubbub" do
+      url = "http://example.com/public/username.atom"
+      m = double
 
-      expect(m).to receive(:publish).with(url+'.atom')
+      expect(m).to receive(:publish).with(url)
       expect(Pubsubhubbub).to receive(:new).with(AppConfig.environment.pubsub_server).and_return(m)
       Workers::PublishToHub.new.perform(url)
     end