github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2024-11-25 08:43:25 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

DEV: Remove old TODOs for message-id formats (#27196)

Browse Source 
Introduced back in 2022 in
e3d495850d,
our new more specific message-id format for inbound and
outbound emails has now been in use for a very long time,
we can remove the support for the old formats:

`topic/:topic_id/:post_id.:random@:host`
`topic/:topic_id@:host`
`topic/:topic_id.:random@:host`
This commit is contained in:
Martin Brennan 2024-05-28 13:57:09 +10:00 committed by GitHub
parent 39902c148f
commit 9c85ea5945
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 19 additions and 121 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
lib/email/message_id_service.rb
View File

@ -71,29 +71,13 @@ module Email
def find_post_from_message_ids(message_ids)
message_ids = message_ids.map { |message_id| message_id_clean(message_id) }
# TODO (martin) 2023-04-01 We should remove these backwards-compatible
# formats for the Message-ID and solely use the discourse/post/999@host
# format.
topic_ids =
message_ids
.map { |message_id| message_id[message_id_topic_id_regexp, 1] }
.compact
.map(&:to_i)
post_ids =
message_ids
.map { |message_id| message_id[message_id_post_id_regexp, 1] }
.compact
.map(&:to_i)
post_ids << message_ids
.map { |message_id| message_id[message_id_discourse_regexp, 1] }
.compact
.map(&:to_i)
post_ids << Post
.where(outbound_message_id: message_ids)
.or(Post.where(topic_id: topic_ids, post_number: 1))
.pluck(:id)
post_ids << Post.where(outbound_message_id: message_ids).pluck(:id)
post_ids << EmailLog.where(message_id: message_ids).pluck(:post_id)
post_ids << IncomingEmail.where(message_id: message_ids).pluck(:post_id)
@ -106,24 +90,8 @@ module Email
Post.where(id: post_ids).order(:created_at).last
end
# TODO (martin) 2023-04-01 We should remove these backwards-compatible
# formats for the Message-ID and solely use the discourse/post/999@host
# format.
def discourse_generated_message_id?(message_id)
!!(message_id =~ message_id_post_id_regexp) ||
!!(message_id =~ message_id_topic_id_regexp) ||
!!(message_id =~ message_id_discourse_regexp)
end
# TODO (martin) 2023-04-01 We should remove these backwards-compatible
# formats for the Message-ID and solely use the discourse/post/999@host
# format.
def message_id_post_id_regexp
Regexp.new "topic/\\d+/(\\d+|\\d+\.\\w+)@#{Regexp.escape(host)}"
end
def message_id_topic_id_regexp
Regexp.new "topic/(\\d+|\\d+\.\\w+)@#{Regexp.escape(host)}"
message_id_discourse_regexp.match?(message_id)
end
def message_id_discourse_regexp

2
spec/fixtures/emails/old_destination.eml vendored
View File

@ -4,7 +4,7 @@ To: reply+4f97315cc828096c9cb34c6f1a0d6fe8@bar.com
Cc: foofoo@bar.com
Date: Fri, 15 Jan 2018 00:12:43 +0100
Message-ID: <999@foo.bar.mail>
In-Reply-To: <topic/424242/123456@test.localhost>
In-Reply-To: <discourse/post/:post_id@test.localhost>
Mime-Version: 1.0
Content-Type: text/plain
Content-Transfer-Encoding: 7bit

5
spec/lib/email/processor_spec.rb
View File

@ -180,10 +180,7 @@ RSpec.describe Email::Processor do
fab!(:topic)
fab!(:post) { Fabricate(:post, topic: topic, created_at: 3.days.ago) }
let(:mail) do
file_from_fixtures("old_destination.eml", "emails")
.read
.gsub("424242", topic.id.to_s)
.gsub("123456", post.id.to_s)
file_from_fixtures("old_destination.eml", "emails").read.gsub(":post_id", post.id.to_s)
end
it "rejects the email with the right response" do

40
spec/lib/email/receiver_spec.rb
View File

@ -93,7 +93,7 @@ RSpec.describe Email::Receiver do
post = Fabricate(:post, topic: topic)
user = Fabricate(:user, email: "discourse@bar.com")
mail = email(:old_destination).gsub("424242", topic.id.to_s)
mail = email(:old_destination).gsub(":post_id", post.id.to_s)
expect { Email::Receiver.new(mail).process! }.to raise_error(
Email::Receiver::BadDestinationAddress,
)
@ -1120,44 +1120,6 @@ RSpec.describe Email::Receiver do
Email::Receiver.new(mail_string).process!
end
it "posts a reply using a message-id in the format topic/TOPIC_ID/POST_ID@HOST" do
expect {
process_mail_with_message_id("topic/#{topic.id}/#{post.id}@test.localhost")
}.to change { Post.count }.by(1)
expect(topic.reload.posts.last.raw).to include(
"This is email reply testing with Message-ID formats",
)
end
it "posts a reply using a message-id in the format topic/TOPIC_ID@HOST" do
expect { process_mail_with_message_id("topic/#{topic.id}@test.localhost") }.to change {
Post.count
}.by(1)
expect(topic.reload.posts.last.raw).to include(
"This is email reply testing with Message-ID formats",
)
end
it "posts a reply using a message-id in the format topic/TOPIC_ID/POST_ID.RANDOM_SUFFIX@HOST" do
expect {
process_mail_with_message_id("topic/#{topic.id}/#{post.id}.rjc3yr79834y@test.localhost")
}.to change { Post.count }.by(1)
expect(topic.reload.posts.last.raw).to include(
"This is email reply testing with Message-ID formats",
)
end
it "posts a reply using a message-id in the format topic/TOPIC_ID.RANDOM_SUFFIX@HOST" do
expect {
process_mail_with_message_id(
"topic/#{topic.id}/#{post.id}.x3487nxy877843x@test.localhost",
)
}.to change { Post.count }.by(1)
expect(topic.reload.posts.last.raw).to include(
"This is email reply testing with Message-ID formats",
)
end
it "posts a reply using a message-id in the format discourse/post/POST_ID@HOST" do
expect {
process_mail_with_message_id("discourse/post/#{post.id}@test.localhost")

2
spec/lib/imap/sync_spec.rb
View File

@ -142,7 +142,7 @@ RSpec.describe Imap::Sync do
end
context "when the message id matches the receiver post id regex" do
let(:message_id) { "topic/999/324@test.localhost" }
let(:message_id) { "discourse/post/324@test.localhost" }
it "does not duplicate incoming email" do
incoming_email = Fabricate(:incoming_email, message_id: message_id)

29
spec/lib/message_id_service_spec.rb
View File

@ -52,22 +52,11 @@ RSpec.describe Email::MessageIdService do
end
describe "find_post_from_message_ids" do
let(:post_format_message_id) { "<topic/#{topic.id}/#{post.id}.test123@test.localhost>" }
let(:topic_format_message_id) { "<topic/#{topic.id}.test123@test.localhost>" }
let(:discourse_format_message_id) { "<discourse/post/#{post.id}@test.localhost>" }
let(:default_format_message_id) { "<36ac1ddd-5083-461d-b72c-6372fb0e7f33@test.localhost>" }
let(:gmail_format_message_id) do
"<CAPGrNgZ7QEFuPcsxJBRZLhBhAYPO_ruYpCANSdqiQEbc9Otpiw@mail.gmail.com>"
end
it "finds a post based only on a post-format message id" do
expect(described_class.find_post_from_message_ids([post_format_message_id])).to eq(post)
end
it "finds a post based only on a topic-format message id" do
expect(described_class.find_post_from_message_ids([topic_format_message_id])).to eq(post)
end
it "finds a post based only on a discourse-format message id" do
expect(described_class.find_post_from_message_ids([discourse_format_message_id])).to eq(post)
end
@ -101,16 +90,6 @@ RSpec.describe Email::MessageIdService do
incoming_email.post,
)
end
it "gets the last created post if multiple are returned" do
incoming_email =
Fabricate(
:incoming_email,
message_id: described_class.message_id_clean(post_format_message_id),
post: Fabricate(:post, created_at: 10.days.ago),
)
expect(described_class.find_post_from_message_ids([post_format_message_id])).to eq(post)
end
end
describe "#discourse_generated_message_id?" do
@ -119,14 +98,6 @@ RSpec.describe Email::MessageIdService do
end
it "works correctly for the different possible formats" do
expect(check_format("topic/1223/4525.3c4f8n9@test.localhost")).to eq(true)
expect(check_format("<topic/1223/4525.3c4f8n9@test.localhost>")).to eq(true)
expect(check_format("topic/1223.fc3j4843@test.localhost")).to eq(true)
expect(check_format("<topic/1223.fc3j4843@test.localhost>")).to eq(true)
expect(check_format("topic/1223/4525@test.localhost")).to eq(true)
expect(check_format("<topic/1223/4525@test.localhost>")).to eq(true)
expect(check_format("topic/1223@test.localhost")).to eq(true)
expect(check_format("<topic/1223@test.localhost>")).to eq(true)
expect(check_format("discourse/post/1223@test.localhost")).to eq(true)
expect(check_format("<discourse/post/1223@test.localhost>")).to eq(true)