nocebo/diaspora
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #8403 from SuperTux88/cleanup-duplicate-pods-for-real-this-time

Browse source 
Cleanup duplicate pods
This commit is contained in:
Benjamin Neff 2022-11-01 21:16:12 +01:00
commit 80c0888176
No known key found for this signature in database
GPG key ID: 971464C3F1A90194
8 changed files with 93 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
Changelog.md
View file

@ -54,6 +54,7 @@ We use yarn to install the frontend dependencies now, so you need to have that i
* Fix fetching public posts on first account search was missing some data [#8390](https://github.com/diaspora/diaspora/pull/8390)
* Add redirect from mobile UI photo URLs to post when not using mobile UI [#8400](https://github.com/diaspora/diaspora/pull/8400)
* Escape mentions before markdown parsing in mobile UI [#8398](https://github.com/diaspora/diaspora/pull/8398)
* Cleanup duplicate pods in database [#8403](https://github.com/diaspora/diaspora/pull/8403)
## Features
* Add client-side cropping of profile image uploads [#7581](https://github.com/diaspora/diaspora/pull/7581)

3
app/assets/javascripts/app/views/pod_entry_view.js
View file

@ -27,12 +27,13 @@ app.views.PodEntry = app.views.Base.extend({
presenter: function() {
return _.extend({}, this.defaultPresenter(), {
/* jshint camelcase: false */
hasPort: (this.model.get("port") >= 0),
is_unchecked: (this.model.get("status")==="unchecked"),
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") +
(this.model.get("port") ? ":" + this.model.get("port") : ""),
(this.model.get("port") >= 0 ? ":" + this.model.get("port") : ""),
response_time_fmt: this._fmtResponseTime()
/* jshint camelcase: true */
});

2
app/assets/templates/pod_table_entry_tpl.jst.hbs
View file

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

28
app/models/pod.rb
View file

@ -22,7 +22,7 @@ class Pod < ApplicationRecord
ConnectionTester::SSLFailure => :ssl_failed,
ConnectionTester::HTTPFailure => :http_failed,
ConnectionTester::NodeInfoFailure => :version_failed
}
}.freeze
# this are only the most common errors, the rest will be +unknown_error+
CURL_ERROR_MAP = {
@ -34,7 +34,13 @@ class Pod < ApplicationRecord
redirected_to_other_hostname: :http_failed
}.freeze
DEFAULT_PORTS = [URI::HTTP::DEFAULT_PORT, URI::HTTPS::DEFAULT_PORT]
# use -1 as port for default ports
# we can't use the real default port (80/443) because we need to handle them
# like both are the same and not both can exist at the same time.
# we also can't use nil, because databases don't handle NULL in unique indexes
# (except postgres >= 15 with "NULLS NOT DISTINCT").
DEFAULT_PORT = -1
DEFAULT_PORTS = [URI::HTTP::DEFAULT_PORT, URI::HTTPS::DEFAULT_PORT].freeze
has_many :people
@ -51,8 +57,8 @@ class Pod < ApplicationRecord
class << self
def find_or_create_by(opts) # Rename this method to not override an AR method
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|
port = DEFAULT_PORTS.include?(uri.port) ? DEFAULT_PORT : uri.port
find_or_initialize_by(host: uri.host.downcase, port: port).tap do |pod|
pod.ssl ||= (uri.scheme == "https")
pod.save
end
@ -147,13 +153,21 @@ class Pod < ApplicationRecord
# @return [URI]
def uri
@uri ||= (ssl ? URI::HTTPS : URI::HTTP).build(host: host, port: port)
@uri ||= (ssl ? URI::HTTPS : URI::HTTP).build(host: host, port: real_port)
@uri.dup
end
def real_port
if port == DEFAULT_PORT
ssl ? URI::HTTPS::DEFAULT_PORT : URI::HTTP::DEFAULT_PORT
else
port
end
end
def not_own_pod
pod_uri = AppConfig.pod_uri
pod_port = DEFAULT_PORTS.include?(pod_uri.port) ? nil : pod_uri.port
errors.add(:base, "own pod not allowed") if pod_uri.host == host && pod_port == port
pod_port = DEFAULT_PORTS.include?(pod_uri.port) ? DEFAULT_PORT : pod_uri.port
errors.add(:base, "own pod not allowed") if pod_uri.host.downcase == host && pod_port == port
end
end

48
db/migrate/20221030193943_cleanup_duplicate_pods.rb Normal file
View file

@ -0,0 +1,48 @@
# frozen_string_literal: true
class CleanupDuplicatePods < ActiveRecord::Migration[6.1]
class Pod < ApplicationRecord
end
def change
reversible do |change|
change.up do
remove_duplicates
cleanup_mixed_case_pods
Pod.where(port: nil).update_all(port: -1) # rubocop:disable Rails/SkipsModelValidations
end
change.down do
Pod.where(port: -1).update_all(port: nil) # rubocop:disable Rails/SkipsModelValidations
end
end
change_column_null :pods, :port, false
end
private
def remove_duplicates
Pod.where(port: nil).group(:host).having("count(*) > 1").pluck(:host).each do |host|
cleanup_duplicates(Pod.where(host: host).order(:id).ids)
end
end
def cleanup_mixed_case_pods
Pod.where("lower(host) != host").pluck(:host, :port).each do |host, port|
pod_ids = Pod.where("lower(host) = ?", host.downcase).where(port: port).order(:id).ids
cleanup_duplicates(pod_ids.dup) if pod_ids.size > 1
Pod.find(pod_ids.first).update(host: host.downcase)
end
end
def cleanup_duplicates(duplicate_ids)
target_pod_id = duplicate_ids.shift
duplicate_ids.each do |pod_id|
Person.where(pod_id: pod_id).update_all(pod_id: target_pod_id) # rubocop:disable Rails/SkipsModelValidations
Pod.delete(pod_id)
end
end
end

1
spec/factories.rb
View file

@ -249,6 +249,7 @@ FactoryBot.define do
factory :pod do
sequence(:host) {|n| "pod#{n}.example#{r_str}.com" }
port { -1 }
ssl { true }
end

4
spec/javascripts/app/views/pod_entry_view_spec.js
View file

@ -30,11 +30,13 @@ describe("app.views.PodEntry", function() {
this.pod.set({
status: "no_errors",
ssl: true,
host: "pod.example.com"
host: "pod.example.com",
port: -1
});
var actual = this.view.presenter();
expect(actual).toEqual(jasmine.objectContaining({
/* jshint camelcase: false */
hasPort: false,
is_unchecked: false,
has_no_errors: true,
has_errors: false,

17
spec/models/pod_spec.rb
View file

@ -16,26 +16,36 @@ describe Pod, type: :model do
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
expect(pod.port).to eq(Pod::DEFAULT_PORT)
end
it "sets ssl boolean" do
pod = Pod.find_or_create_by(url: "https://example.org/")
expect(pod.ssl).to be true
expect(pod.port).to eq(Pod::DEFAULT_PORT)
end
it "updates ssl boolean if upgraded to https" do
pod = Pod.find_or_create_by(url: "http://example.org/")
expect(pod.ssl).to be false
expect(pod.port).to eq(Pod::DEFAULT_PORT)
pod = Pod.find_or_create_by(url: "https://example.org/")
expect(pod.ssl).to be true
expect(pod.port).to eq(Pod::DEFAULT_PORT)
end
it "does not update ssl boolean if downgraded to http" do
pod = Pod.find_or_create_by(url: "https://example.org/")
expect(pod.ssl).to be true
expect(pod.port).to eq(Pod::DEFAULT_PORT)
pod = Pod.find_or_create_by(url: "http://example.org/")
expect(pod.ssl).to be true
expect(pod.port).to eq(Pod::DEFAULT_PORT)
end
it "normalizes hostname to lowercase" do
pod = Pod.find_or_create_by(url: "https://eXaMpLe.oRg/")
expect(pod.host).to eq("example.org")
end
context "validation" do
@ -205,6 +215,11 @@ describe Pod, type: :model do
pod = FactoryBot.create(:pod)
expect(pod.url_to("/receive/public")).to eq("https://#{pod.host}/receive/public")
end
it "includes non-default port in pod url" do
pod = FactoryBot.create(:pod, port: 3000)
expect(pod.url_to("/receive/public")).to eq("https://#{pod.host}:#{pod.port}/receive/public")
end
end
describe "#update_offline_since" do