Remove some code in TagManager and spec (#3547)

* Do not fall back to StreamEntry if object_type is unavailable in TagManager

Since 6d6a429af8, when Status, the only model
with stream_entry, and StreamEntry got its own logic in uri_for and
url_for, the purpose of the fallbacks to activity_type of StreamEntry
became unclear.

This commit removes the fallbacks. When adding another model with
stream_entry in future, consider to update uri_for and url_for.

* Cover TagManager more
This commit is contained in:
Akihiko Odaki (@fn_aki@pawoo.net) 2017-06-04 23:14:25 +09:00 committed by Eugen Rochko
parent 7c7c18fdea
commit e07b57852e
2 changed files with 193 additions and 32 deletions

View file

@ -93,8 +93,6 @@ class TagManager
account_url(target) account_url(target)
when :note, :comment, :activity when :note, :comment, :activity
unique_tag(target.created_at, target.id, 'Status') unique_tag(target.created_at, target.id, 'Status')
else
unique_tag(target.stream_entry.created_at, target.stream_entry.activity_id, target.stream_entry.activity_type)
end end
end end
@ -106,8 +104,6 @@ class TagManager
short_account_url(target) short_account_url(target)
when :note, :comment, :activity when :note, :comment, :activity
short_account_status_url(target.account, target) short_account_status_url(target.account, target)
else
account_stream_entry_url(target.account, target.stream_entry)
end end
end end
end end

View file

@ -1,23 +1,152 @@
require 'rails_helper' require 'rails_helper'
RSpec.describe TagManager do RSpec.describe TagManager do
let(:local_domain) { Rails.configuration.x.local_domain } describe '#local_domain?' do
# The following comparisons MUST be case-insensitive.
around do |example|
original_local_domain = Rails.configuration.x.local_domain
Rails.configuration.x.local_domain = 'domain'
example.run
Rails.configuration.x.local_domain = original_local_domain
end
it 'returns true for nil' do
expect(TagManager.instance.local_domain?(nil)).to eq true
end
it 'returns true if the slash-stripped string equals to local domain' do
expect(TagManager.instance.local_domain?('DoMaIn/')).to eq true
end
it 'returns false for irrelevant string' do
expect(TagManager.instance.local_domain?('DoMaIn!')).to eq false
end
end
describe '#web_domain?' do
# The following comparisons MUST be case-insensitive.
around do |example|
original_web_domain = Rails.configuration.x.web_domain
Rails.configuration.x.web_domain = 'domain'
example.run
Rails.configuration.x.web_domain = original_web_domain
end
it 'returns true for nil' do
expect(TagManager.instance.web_domain?(nil)).to eq true
end
it 'returns true if the slash-stripped string equals to web domain' do
expect(TagManager.instance.web_domain?('DoMaIn/')).to eq true
end
it 'returns false for string with irrelevant characters' do
expect(TagManager.instance.web_domain?('DoMaIn!')).to eq false
end
end
describe '#normalize_domain' do
it 'returns nil if the given parameter is nil' do
expect(TagManager.instance.normalize_domain(nil)).to eq nil
end
it 'returns normalized domain' do
expect(TagManager.instance.normalize_domain('DoMaIn/')).to eq 'domain'
end
end
describe '#local_url?' do
around do |example|
original_local_domain = Rails.configuration.x.local_domain
example.run
Rails.configuration.x.local_domain = original_local_domain
end
it 'returns true if the normalized string with port is local URL' do
Rails.configuration.x.local_domain = 'domain:42'
expect(TagManager.instance.local_url?('https://DoMaIn:42/')).to eq true
end
it 'returns true if the normalized string without port is local URL' do
Rails.configuration.x.local_domain = 'domain'
expect(TagManager.instance.local_url?('https://DoMaIn/')).to eq true
end
it 'returns false for string with irrelevant characters' do
Rails.configuration.x.local_domain = 'domain'
expect(TagManager.instance.local_url?('https://domainn/')).to eq false
end
end
describe '#same_acct?' do
# The following comparisons MUST be case-insensitive.
it 'returns true if the needle has a correct username and domain for remote user' do
expect(TagManager.instance.same_acct?('username@domain', 'UsErNaMe@DoMaIn')).to eq true
end
it 'returns false if the needle is missing a domain for remote user' do
expect(TagManager.instance.same_acct?('username@domain', 'UsErNaMe')).to eq false
end
it 'returns false if the needle has an incorrect domain for remote user' do
expect(TagManager.instance.same_acct?('username@domain', 'UsErNaMe@incorrect')).to eq false
end
it 'returns false if the needle has an incorrect username for remote user' do
expect(TagManager.instance.same_acct?('username@domain', 'incorrect@DoMaIn')).to eq false
end
it 'returns true if the needle has a correct username and domain for local user' do
expect(TagManager.instance.same_acct?('username', 'UsErNaMe@Cb6E6126.nGrOk.Io')).to eq true
end
it 'returns true if the needle is missing a domain for local user' do
expect(TagManager.instance.same_acct?('username', 'UsErNaMe')).to eq true
end
it 'returns false if the needle has an incorrect username for local user' do
expect(TagManager.instance.same_acct?('username', 'UsErNaM@Cb6E6126.nGrOk.Io')).to eq false
end
it 'returns false if the needle has an incorrect domain for local user' do
expect(TagManager.instance.same_acct?('username', 'incorrect@Cb6E6126.nGrOk.Io')).to eq false
end
end
describe '#unique_tag' do describe '#unique_tag' do
it 'returns a string' do it 'returns a unique tag' do
expect(TagManager.instance.unique_tag(Time.now, 12, 'Status')).to be_a String expect(TagManager.instance.unique_tag(Time.utc(2000), 12, 'Status')).to eq 'tag:cb6e6126.ngrok.io,2000-01-01:objectId=12:objectType=Status'
end end
end end
describe '#unique_tag_to_local_id' do describe '#unique_tag_to_local_id' do
it 'returns the ID part' do it 'returns the ID part' do
expect(TagManager.instance.unique_tag_to_local_id("tag:#{local_domain};objectId=12:objectType=Status", 'Status')).to eql '12' expect(TagManager.instance.unique_tag_to_local_id('tag:cb6e6126.ngrok.io,2000-01-01:objectId=12:objectType=Status', 'Status')).to eql '12'
end
it 'returns nil if it is not local id' do
expect(TagManager.instance.unique_tag_to_local_id('tag:remote,2000-01-01:objectId=12:objectType=Status', 'Status')).to eq nil
end
it 'returns nil if it is not expected type' do
expect(TagManager.instance.unique_tag_to_local_id('tag:cb6e6126.ngrok.io,2000-01-01:objectId=12:objectType=Block', 'Status')).to eq nil
end
it 'returns nil if it does not have object ID' do
expect(TagManager.instance.unique_tag_to_local_id('tag:cb6e6126.ngrok.io,2000-01-01:objectType=Status', 'Status')).to eq nil
end end
end end
describe '#local_id?' do describe '#local_id?' do
it 'returns true for a local ID' do it 'returns true for a local ID' do
expect(TagManager.instance.local_id?("tag:#{local_domain};objectId=12:objectType=Status")).to be true expect(TagManager.instance.local_id?('tag:cb6e6126.ngrok.io;objectId=12:objectType=Status')).to be true
end end
it 'returns false for a foreign ID' do it 'returns false for a foreign ID' do
@ -26,49 +155,85 @@ RSpec.describe TagManager do
end end
describe '#uri_for' do describe '#uri_for' do
let(:alice) { Fabricate(:account, username: 'alice') }
let(:bob) { Fabricate(:account, username: 'bob') }
let(:status) { Fabricate(:status, text: 'Hello world', account: alice) }
subject { TagManager.instance.uri_for(target) } subject { TagManager.instance.uri_for(target) }
context 'Account' do context 'activity object' do
let(:target) { alice } let(:target) { Fabricate(:status, reblog: Fabricate(:status)).stream_entry }
it 'returns a string' do before { target.update!(created_at: '2000-01-01T00:00:00Z') }
expect(subject).to be_a String
it 'returns the unique tag for status' do
expect(target.object_type).to eq :activity
is_expected.to eq "tag:cb6e6126.ngrok.io,2000-01-01:objectId=#{target.id}:objectType=Status"
end end
end end
context 'Status' do context 'comment object' do
let(:target) { status } let(:target) { Fabricate(:status, created_at: '2000-01-01T00:00:00Z', reply: true) }
it 'returns a string' do it 'returns the unique tag for status' do
expect(subject).to be_a String expect(target.object_type).to eq :comment
is_expected.to eq "tag:cb6e6126.ngrok.io,2000-01-01:objectId=#{target.id}:objectType=Status"
end
end
context 'note object' do
let(:target) { Fabricate(:status, created_at: '2000-01-01T00:00:00Z', reply: false, thread: nil) }
it 'returns the unique tag for status' do
expect(target.object_type).to eq :note
is_expected.to eq "tag:cb6e6126.ngrok.io,2000-01-01:objectId=#{target.id}:objectType=Status"
end
end
context 'person object' do
let(:target) { Fabricate(:account, username: 'alice') }
it 'returns the URL for account' do
expect(target.object_type).to eq :person
is_expected.to eq 'https://cb6e6126.ngrok.io/users/alice'
end end
end end
end end
describe '#url_for' do describe '#url_for' do
let(:alice) { Fabricate(:account, username: 'alice') } let(:alice) { Fabricate(:account, username: 'alice') }
let(:bob) { Fabricate(:account, username: 'bob') }
let(:status) { Fabricate(:status, text: 'Hello world', account: alice) }
subject { TagManager.instance.url_for(target) } subject { TagManager.instance.url_for(target) }
context 'Account' do context 'activity object' do
let(:target) { alice } let(:target) { Fabricate(:status, account: alice, reblog: Fabricate(:status)).stream_entry }
it 'returns a URL' do it 'returns the unique tag for status' do
expect(subject).to be_a String expect(target.object_type).to eq :activity
is_expected.to eq "https://cb6e6126.ngrok.io/@alice/#{target.id}"
end end
end end
context 'Status' do context 'comment object' do
let(:target) { status } let(:target) { Fabricate(:status, account: alice, reply: true) }
it 'returns a URL' do it 'returns the unique tag for status' do
expect(subject).to be_a String expect(target.object_type).to eq :comment
is_expected.to eq "https://cb6e6126.ngrok.io/@alice/#{target.id}"
end
end
context 'note object' do
let(:target) { Fabricate(:status, account: alice, reply: false, thread: nil) }
it 'returns the unique tag for status' do
expect(target.object_type).to eq :note
is_expected.to eq "https://cb6e6126.ngrok.io/@alice/#{target.id}"
end
end
context 'person object' do
let(:target) { alice }
it 'returns the URL for account' do
expect(target.object_type).to eq :person
is_expected.to eq 'https://cb6e6126.ngrok.io/@alice'
end end
end end
end end