Ruby 3에서 TopicEmbed.import 스펙 중 하나가 실패합니다

yahonda · 10월 6, 2021, 2:31오후

Ruby 3에서 실패하는 TopicEmbed.import 스펙에 대해 도움을 받고 싶습니다.

재현 방법

cd discourse
bundle exec rspec ./spec/models/topic_embed_spec.rb:154

기대 동작

성공적으로 완료되어야 합니다.

Ruby 3.0.2에서의 실제 동작

$ bundle exec rspec ./spec/models/topic_embed_spec.rb:154
Run options: include {:locations=>{"./spec/models/topic_embed_spec.rb"=>[154]}}

Randomized with seed 41701
F

Failures:

  1) TopicEmbed.import embedded content truncation keeps everything in the imported post when truncation is disabled
     Failure/Error: expect(post.raw).to include(long_content)

       expected "<p>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...for the original entry at <a href=\"http://eviltrout.com/123\">http://eviltrout.com/123</a></small>" to include "<p>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...for the original entry at <a href='http://eviltrout.com/123'>http://eviltrout.com/123</a></small>\n"
       Diff:
       @@ -1,4 +1,7 @@
       -<p>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa</p>\n<p>more</p>\n<hr>\n<small>This is a companion discussion topic for the original entry at <a href='http://eviltrout.com/123'>http://eviltrout.com/123</a></small>\n
       +<p>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa</p>
       +<p>more</p>
       +<hr>
       +<small>This is a companion discussion topic for the original entry at <a href="http://eviltrout.com/123">http://eviltrout.com/123</a></small>

     # ./spec/models/topic_embed_spec.rb:158:in `block (4 levels) in <top (required)>'
     # ./spec/rails_helper.rb:279:in `block (2 levels) in <top (required)>'

Finished in 2.48 seconds (files took 2.73 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./spec/models/topic_embed_spec.rb:154 # TopicEmbed.import embedded content truncation keeps everything in the imported post when truncation is disabled

Randomized with seed 41701

$

Ruby 2.7.4에서의 실제 동작

$ ruby -v
ruby 2.7.4p191 (2021-07-07 revision a21a3b7d23) [x86_64-linux]
$ bundle exec rspec ./spec/models/topic_embed_spec.rb:154
Run options: include {:locations=>{"./spec/models/topic_embed_spec.rb"=>[154]}}

Randomized with seed 4126
.

Finished in 2.51 seconds (files took 2.31 seconds to load)
1 example, 0 failures

Randomized with seed 4126

$

지금까지 발견한 내용

일부 print 디버그 코드를 추가하여 topic_embed_spec.rb 안의 long_content가 app/models/topic_embed.rb의 contents에 의해 덮어쓰기되는 것을 발견했습니다.

Print 디버그 코드

$ git diff
diff --git a/app/models/topic_embed.rb b/app/models/topic_embed.rb
index 1e9f06a322..ab407d8005 100644
--- a/app/models/topic_embed.rb
+++ b/app/models/topic_embed.rb
@@ -28,6 +28,7 @@ class TopicEmbed < ActiveRecord::Base

   # Import an article from a source (RSS/Atom/Other)
   def self.import(user, url, title, contents, category_id: nil, cook_method: nil, tags: nil)
+    p "contents.length at the begining of self.import: #{contents.length}"
     return unless url =~ /^https?\:\/\//

     if SiteSetting.embed_truncate && cook_method.nil?
@@ -102,6 +103,7 @@ class TopicEmbed < ActiveRecord::Base
       end
     end

+    p "contents.length at the end of self.import: #{contents.length}"
     post
   end

diff --git a/spec/models/topic_embed_spec.rb b/spec/models/topic_embed_spec.rb
index 8659eb7d56..1266441da7 100644
--- a/spec/models/topic_embed_spec.rb
+++ b/spec/models/topic_embed_spec.rb
@@ -153,7 +153,9 @@ describe TopicEmbed do

       it 'keeps everything in the imported post when truncation is disabled' do
         SiteSetting.embed_truncate = false
+        p "long_content.length before calling TopicEmbed.import: #{long_content.length}"
         post = TopicEmbed.import(user, url, title, long_content)
+        p "long_content.length after calling TopicEmbed.import: #{long_content.length}"

         expect(post.raw).to include(long_content)
       end
$

Ruby 3.0.2에서의 결과

$ bundle exec rspec ./spec/models/topic_embed_spec.rb:154
Run options: include {:locations=>{"./spec/models/topic_embed_spec.rb"=>[154]}}

Randomized with seed 24988
"long_content.length before calling TopicEmbed.import: 119"
"contents.length at the begining of self.import: 119"
"contents.length at the end of self.import: 267"
"long_content.length after calling TopicEmbed.import: 267"
F

Failures:

  1) TopicEmbed.import embedded content truncation keeps everything in the imported post when truncation is disabled
     Failure/Error: expect(post.raw).to include(long_content)

       expected "<p>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...for the original entry at <a href=\"http://eviltrout.com/123\">http://eviltrout.com/123</a></small>" to include "<p>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...for the original entry at <a href='http://eviltrout.com/123'>http://eviltrout.com/123</a></small>\n"
       Diff:
       @@ -1,4 +1,7 @@
       -<p>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa</p>\n<p>more</p>\n<hr>\n<small>This is a companion discussion topic for the original entry at <a href='http://eviltrout.com/123'>http://eviltrout.com/123</a></small>\n
       +<p>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa</p>
       +<p>more</p>
       +<hr>
       +<small>This is a companion discussion topic for the original entry at <a href="http://eviltrout.com/123">http://eviltrout.com/123</a></small>

     # ./spec/models/topic_embed_spec.rb:160:in `block (4 levels) in <top (required)>'
     # ./spec/rails_helper.rb:279:in `block (2 levels) in <top (required)>'

Finished in 2.55 seconds (files took 3.4 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./spec/models/topic_embed_spec.rb:154 # TopicEmbed.import embedded content truncation keeps everything in the imported post when truncation is disabled

Randomized with seed 24988

$

Ruby 2.7.4에서의 결과

$ bundle exec rspec ./spec/models/topic_embed_spec.rb:154
Run options: include {:locations=>{"./spec/models/topic_embed_spec.rb"=>[154]}}

Randomized with seed 1802
"long_content.length before calling TopicEmbed.import: 119"
"contents.length at the begining of self.import: 119"
"contents.length at the end of self.import: 267"
"long_content.length after calling TopicEmbed.import: 119"
.

Finished in 2.56 seconds (files took 3.06 seconds to load)
1 example, 0 failures

Randomized with seed 1802

$

yahonda · 10월 8, 2021, 6:00오전

왜 이 스펙이 Ruby 3.0.2에서만 실패하는지 이유를 찾았습니다.

Ruby 3부터는 frozen-string-literal 마법 주석이 활성화되어 있더라도 보간된 문자열은 동결되지 않습니다.

따라서 이를 수정할 수 있는 방법은 다음과 같습니다.

+contents 대신 contents.dup 사용

github.com/discourse/discourse

app/models/topic_embed.rb

016524e24


      
          end
          
          # Import an article from a source (RSS/Atom/Other)
          def self.import(user, url, title, contents, category_id: nil, cook_method: nil, tags: nil)
            return unless url =~ /^https?\:\/\//
          
            if SiteSetting.embed_truncate && cook_method.nil?
              contents = first_paragraph_from(contents)
            end
            contents ||= ''
            contents = +contents << imported_from_html(url)
          
            url = normalize_url(url)
          
            embed = TopicEmbed.find_by("lower(embed_url) = ?", url)
            content_sha1 = Digest::SHA1.hexdigest(contents)
            post = nil
          
            # If there is no embed, create a topic, post and the embed.
            if embed.blank?
              Topic.transaction do

$ git diff
diff --git a/app/models/topic_embed.rb b/app/models/topic_embed.rb
index 1e9f06a322..502409d3c1 100644
--- a/app/models/topic_embed.rb
+++ b/app/models/topic_embed.rb
@@ -34,7 +34,7 @@ class TopicEmbed < ActiveRecord::Base
       contents = first_paragraph_from(contents)
     end
     contents ||= ''
-    contents = +contents << imported_from_html(url)
+    contents = contents.dup << imported_from_html(url)

     url = normalize_url(url)

$

보간된 long_content 동결

github.com/discourse/discourse

spec/models/topic_embed_spec.rb

016524e24


      
              expect(post).to be_present
          
              # It uses regular rendering
              expect(post.cook_method).to eq(Post.cook_methods[:regular])
            end
          end
          
          describe 'embedded content truncation' do
            MAX_LENGTH_BEFORE_TRUNCATION = 100
          
            let(:long_content) { "<p>#{'a' * MAX_LENGTH_BEFORE_TRUNCATION}</p>\n<p>more</p>" }
          
            it 'truncates the imported post when truncation is enabled' do
              SiteSetting.embed_truncate = true
              post = TopicEmbed.import(user, url, title, long_content)
          
              expect(post.raw).not_to include(long_content)
            end
          
            it 'keeps everything in the imported post when truncation is disabled' do
              SiteSetting.embed_truncate = false

diff --git a/spec/models/topic_embed_spec.rb b/spec/models/topic_embed_spec.rb
index 8659eb7d56..a25351a586 100644
--- a/spec/models/topic_embed_spec.rb
+++ b/spec/models/topic_embed_spec.rb
@@ -142,7 +142,7 @@ describe TopicEmbed do
     describe 'embedded content truncation' do
       MAX_LENGTH_BEFORE_TRUNCATION = 100

-      let(:long_content) { "<p>#{'a' * MAX_LENGTH_BEFORE_TRUNCATION}</p>\n<p>more</p>" }
+      let(:long_content) { "<p>#{'a' * MAX_LENGTH_BEFORE_TRUNCATION}</p>\n<p>more</p>".freeze }

       it 'truncates the imported post when truncation is enabled' do
         SiteSetting.embed_truncate = true
$

contents.dup을 사용하도록 풀 리퀘스트를 여는 쪽에 기울어져 있으며, 해당 아이디어가 구현되었으나 Revert "FIX: Use #dup instead of #+@ since content could be an instan… · discourse/discourse@d01c938 · GitHub 을 통해 되돌려진 것을 발견했습니다.

되돌림의 다른 배경은 찾지 못했는데, 기여자/유지보수자분들의 의견을 듣고 싶습니다.

yahonda · 10월 10, 2021, 1:23오후

FIX: Ruby 3 does not freeze interpolated string - Pull Request #14567 - discourse/discourse - GitHub 열림

글		댓글	조회수
Embed poll_feed fail with UTF8 title Bug	4	1616	4월 27, 2015
phpBB 3 Importer (old) Feature	89	18290	10월 8, 2014
TopicEmbed.embed_url length limit 255 characters Bug	2	1051	4월 21, 2015
Embedding comments widget on localhost causes FrozenError Product Support	1	486	5월 16, 2020
"Show Full Post" button doesn't work in subfolder installations Product Support embedding , subfolder	33	1312	1월 2, 2026