Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

アクセス可能なリンクがリンク切れと判定されないようにする #4046

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
67f0abf
テストしやすくするために Checker にチェック対象リンクを引数で渡せるようにした
AudioStakes Jan 31, 2022
824de67
リンク切れの表記(error/missing_link)を broken_link に統一した
AudioStakes Jan 30, 2022
579c43a
マークダウンからリンクを抽出する処理を Checker クラスから取り除いた
AudioStakes Jan 30, 2022
9617aad
extractor_test.rb で作成している Page のテストデータをフィクスチャへ移した
AudioStakes Jan 29, 2022
9873ba8
Extractor を初期化する時の引数としてドキュメントのみを渡すようにした
AudioStakes Jan 30, 2022
12084f9
テストできるようにするため URL が valid であることの確認をメソッドにした
AudioStakes Jan 30, 2022
3f26c08
テストできるようにするため URL の host がリンク切れ判定対象外か確認するメソッドを追加
AudioStakes Jan 30, 2022
e3e2304
Check.valid_url? のテスト対象に非ASCII(日本語)の URL を追加した
AudioStakes Jan 30, 2022
2ab1ac0
非 ASCII(日本語) を含む URL のパースを gem の Addressable で対応
AudioStakes Jan 28, 2022
cbc5bf7
非 ASCII(日本語) の URL を Addressable#normalize でエンコード
AudioStakes Jan 28, 2022
471321e
スレッドのキュー を Checker の インスタンス変数として持たせるようにした
AudioStakes Jan 28, 2022
f0afbb8
末尾が `)` の URL を正規表現で完全一致できることを確認するテストを追加した
AudioStakes Jan 30, 2022
111d4d4
末尾が `)` の URL を正規表現で完全一致できるようにした
AudioStakes Jan 26, 2022
9e2536c
Extractor クラスの正規表現を URL もしくはパスのみにマッチするように修正した
AudioStakes Jan 30, 2022
2dede3e
テキストから抽出した URL のバリデーションを Extractor クラスで行わないようにした
AudioStakes Jan 28, 2022
a88c60e
SSLサーバー証明書の検証に失敗するサイトのレスポンスも確認できることをテストを追加
AudioStakes Jan 31, 2022
bf1429a
SSLサーバー証明書の検証に失敗するサイトにアクセスするときは証明書を検証しないようにした
AudioStakes Jan 31, 2022
1e6cbde
Amazon の URL をリンク切れ判定の対象外とする理由をコメントした
AudioStakes Jan 27, 2022
03d357e
リンク切れの通知先チャンネルを「バグ、誤字脱字報告、機能要望🙇」へ変更できるようにした
AudioStakes Jan 28, 2022
ba1c01f
Checker クラスの使われていないインスタンス変数を削除した
AudioStakes Jan 31, 2022
579d713
リンクの URL とタイトルの区切り文字 `|` が URL の一部として解釈されないようにした
AudioStakes Jan 31, 2022
54625c2
development モードでのリンク切れチェッカーの動作確認用にフィクスチャを追加
AudioStakes Jan 31, 2022
b4953cd
リンク切れ通知用の path へアクセスしたとき通知内容と同じテキストが画面に表示されるようにした
AudioStakes Feb 1, 2022
c9d9f83
Tables Generator のSSLサーバー証明書を検証しない理由をコメントした
AudioStakes Feb 1, 2022
e73e103
LinkChecker::Extractor クラスをモジュールにした
AudioStakes Feb 3, 2022
4707a61
LinkChecker::Checker クラスをモジュールにした
AudioStakes Feb 3, 2022
cb8e73e
LinkChecker::Link クラスを一つのファイルで定義するようにした
AudioStakes Feb 3, 2022
4309569
Checker.summary のテストを追加
AudioStakes Feb 3, 2022
4036231
Link クラスの文字列表現と<=>演算子を Link クラスに定義した
AudioStakes Feb 3, 2022
09e20c9
pages のフィクスチャに published_at の値を設定した
AudioStakes Feb 7, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ gem 'active_flag'
gem 'active_storage_validations'
gem 'acts_as_list'
gem 'acts-as-taggable-on', '~> 7.0'
gem 'addressable'
gem 'any_login'
gem 'cocoon'
gem 'coffee-rails', '~> 5.0.0'
Expand Down
1 change: 1 addition & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,7 @@ DEPENDENCIES
active_storage_validations
acts-as-taggable-on (~> 7.0)
acts_as_list
addressable
any_login
bootsnap (>= 1.4.4)
bullet
Expand Down
10 changes: 7 additions & 3 deletions app/controllers/scheduler/link_checker_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@

class Scheduler::LinkCheckerController < SchedulerController
def show
checker = LinkChecker::Checker.new
checker.notify_missing_links
render plain: checker.errors.join("\n")
documents = Page.all + Practice.all
links = LinkChecker::Extractor.extract_links_from_multi(documents)
summary_of_broken_links = LinkChecker::Checker.check_broken_links(links)

ChatNotifier.message(summary_of_broken_links, username: 'リンクチェッカー', webhook_url: ENV['DISCORD_BUG_WEBHOOK_URL'])

render plain: summary_of_broken_links
end
end
97 changes: 31 additions & 66 deletions app/models/link_checker/checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,90 +3,55 @@
require 'net/http'

module LinkChecker
class Checker
DENY_LIST = %w[
codepen.io
www.amazon.co.jp
module Checker
DENY_HOST = [
'codepen.io',
'www.amazon.co.jp' # アクセスを繰り返すとリンク切れ判定のレスポンスが返されるようになるため
].freeze
attr_reader :errors

def initialize
@errors = []
@error_links = []
end
module_function

def notify_missing_links
check
return if @error_links.empty?
def check_broken_links(links)
links_with_valid_url = links.select { |link| valid_url?(link.url) && !denied_host?(link.url) }
links_with_response = check_response(links_with_valid_url)
broken_links = links_with_response.select { |link| !link.response || link.response > 403 }
Comment on lines +15 to +17
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checker#check の中で以下の3つの処理が行われており、メソッドの責務が大きいと思ったため、それぞれ分けて行うようにしました。

  • 「url が有効」 かつ「DENY_HOST に含まれない」 link(レスポンス確認対象の link)の判定
  • link の url のレスポンスの確認
  • リンク切れしている link の選別


texts = ['リンク切れがありました。']
@error_links.map do |link|
texts << "- <#{link.url}|#{link.title}> in: <#{link.source_url}|#{link.source_title}>"
end
summary(broken_links)
end

ChatNotifier.message(texts.join("\n"), username: 'リンクチェッカー')
def valid_url?(url)
uri = Addressable::URI.parse(url)
uri.scheme && uri.host
rescue Addressable::URI::InvalidURIError
false
end

def check
def denied_host?(url)
uri = Addressable::URI.parse(url)
DENY_HOST.include?(uri.host)
end

def check_response(links)
locks = Queue.new
5.times { locks.push :lock }
all_links.reject! do |link|
url = URI.encode_www_form_component(link.url)
uri = URI.parse(url)

!uri || DENY_LIST.include?(uri.host)
end
all_links.map do |link|
links.each do |link|
Thread.new do
lock = locks.pop
response = Client.request(link.url)
link.response = response
@error_links << link if !response || response > 403
link.response = Client.request(link.url)
locks.push lock
end
end.each(&:join)

@error_links.sort { |a, b| b.source_url <=> a.source_url }
end

def all_links
page_links + practice_links
end

private

def page_links
links = []
Page.order(:created_at).each do |page|
extractor = Extractor.new(
page.body,
page.title,
"https://bootcamp.fjord.jp#{Rails.application.routes.url_helpers.polymorphic_path(page)}"
)
links += extractor.extract
end.join
end

links
end

def practice_links
links = []
Practice.order(:created_at).each do |practice|
practice_url = Rails.application.routes.url_helpers.polymorphic_path(practice)
extractor = Extractor.new(
practice.description,
practice.title,
"https://bootcamp.fjord.jp#{practice_url}"
)
links += extractor.extract
def summary(broken_links)
return if broken_links.empty?

extractor = Extractor.new(
practice.goal,
practice.title,
"https://bootcamp.fjord.jp#{practice_url}"
)
links += extractor.extract
end
links
texts = ['リンク切れがありました。']
texts << broken_links.sort.map(&:to_s)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link クラスの sort 処理(並び順の判定ロジック)と文字列表現( #to_s )を Link クラス自身に持たせ、Checker クラスはそのメソッドを呼び出すようにしました

texts.join("\n")
end
end
end
25 changes: 11 additions & 14 deletions app/models/link_checker/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

module LinkChecker
class Client
SSL_VERIFY_NONE_HOST = [
'www.tablesgenerator.com' # 中間証明書を取得できず、SSLサーバー証明書の検証に失敗するため
].freeze

def self.request(url)
new(url).request
end
Expand All @@ -11,22 +15,15 @@ def initialize(url)
end

def request
@url = encode_ja(@url)
uri = URI.parse(@url)
response = Net::HTTP.get_response(uri)
response.code.to_i
uri = Addressable::URI.parse(@url).normalize
options = {}
options[:ssl_verify_mode] = OpenSSL::SSL::VERIFY_NONE if SSL_VERIFY_NONE_HOST.include?(uri.host)
response = OpenURI.open_uri(uri, **options)
response.status.first.to_i
rescue OpenURI::HTTPError => e
e.io.status.first.to_i
rescue StandardError => _e
false
end

def encode_ja(url)
url.split(//).map do |c|
if c.match?(%r{[-_.!~*'()a-zA-Z0-9;/?:@&=+$,%#]})
c
else
CGI.escape(c)
end
end.join
end
end
end
31 changes: 13 additions & 18 deletions app/models/link_checker/extractor.rb
Original file line number Diff line number Diff line change
@@ -1,28 +1,23 @@
# frozen_string_literal: true

module LinkChecker
Link = Struct.new(:title, :url, :source_title, :source_url, :response)
module Extractor
MARKDOWN_LINK_REGEXP = %r{\[(.*?)\]\((#{URI::DEFAULT_PARSER.make_regexp}|/.*?)\)}.freeze

class Extractor
def initialize(markdown_text, source_title, source_url)
@markdown_text = markdown_text
@source_title = source_title
@source_url = source_url
module_function

def extract_links_from_multi(documents)
documents.flat_map { |document| extract_links_from_a(document) }
end

def extract
links = @markdown_text.scan(/\[(.*?)\]\((.+?)\)/)&.map do |match|
title = match[0].strip
url = match[1].strip
if url.match?(%r{^/})
uri = URI(@source_url)
uri.path = ''
url = uri.to_s + url
end
Link.new(title, url, @source_title, @source_url)
end
def extract_links_from_a(document)
document.body.scan(MARKDOWN_LINK_REGEXP).map do |title, url_or_path|
title = title.strip
url_or_path = url_or_path.strip
url_or_path = "https://bootcamp.fjord.jp#{url_or_path}" if url_or_path.match?(%r{^/})

links.select { |link| URI::DEFAULT_PARSER.make_regexp.match(link.url) }
Link.new(title, url_or_path, document.title, "https://bootcamp.fjord.jp#{document.path}")
end
end
end
end
13 changes: 13 additions & 0 deletions app/models/link_checker/link.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# frozen_string_literal: true

module LinkChecker
Link = Struct.new(:title, :url, :source_title, :source_url, :response) do
def to_s
"- <#{url} | #{title}> in: <#{source_url} | #{source_title}>"
end

def <=>(other)
(source_url <=> other.source_url).nonzero? || url <=> other.url
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check.summary のテストを常に成功させるため、 link の並び順を決める(比較する)ときに source_url だけでなく url も比較できるようにしました
※ 修正前は source_url のみで比較していたため、source_url が同じ link 同士の並び順が不安定でした

end
end
end
4 changes: 4 additions & 0 deletions app/models/practice.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ def all_text
[title, description, goal].join("\n")
end

def body
[description, goal].join("\n")
end

def product(user)
products.find_by(user: user)
end
Expand Down
1 change: 1 addition & 0 deletions cloudbuild.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ steps:
- '--set-env-vars=DISCORD_NOTICE_WEBHOOK_URL=$_DISCORD_NOTICE_WEBHOOK_URL'
- '--set-env-vars=DISCORD_ALL_WEBHOOK_URL=$_DISCORD_ALL_WEBHOOK_URL'
- '--set-env-vars=DISCORD_ADMIN_WEBHOOK_URL=$_DISCORD_ADMIN_WEBHOOK_URL'
- '--set-env-vars=DISCORD_BUG_WEBHOOK_URL=$_DISCORD_BUG_WEBHOOK_URL'
- '--set-env-vars=BASIC_AUTH_USER=$_BASIC_AUTH_USER'
- '--set-env-vars=BASIC_AUTH_PASSWORD=$_BASIC_AUTH_PASSWORD'
- '--set-env-vars=ROLLBAR_CLIENT_TOKEN=$_ROLLBAR_CLIENT_TOKEN'
Expand Down
27 changes: 26 additions & 1 deletion db/fixtures/pages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ page5:
title: WIPのテスト
body: WIP
user: komagata
published_at: "2021-04-01 00:00:00"

page6:
title: ヘルプのページ
Expand Down Expand Up @@ -78,3 +77,29 @@ page11:
user: komagata
practice: practice1
published_at: "2021-10-01 00:00:00"

page12:
title: apt
body: |-
aptとはdebianでソフトウェアをネットワークからインストールするコマンドです。
[TEST](/test)(/test2)
[missing](test)
- 参考
- [APT - Wikipedia](http://ja.wikipedia.org/wiki/APT)
## Q&A
- Q. `$ apt-cache search vim` の検索結果が多すぎる
- A. [正規表現](https://ja.wikipedia.org/wiki/%E6%AD%A3%E8%A6%8F%E8%A1%A8%E7%8F%BE) を使う。
- 完全一致: `$ apt-cache search ^vim$`
- 前方一致: `$ apt-cache search ^vim`
user: komagata
published_at: "2022-01-01 00:00:00"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

published_atの記載がないと正しく表示されないケースがあるかもしれません。

page13:
title: リンク切れチェッカーのテスト用リンクを載せたページ
body: |-
[リンク切れ判定対象外の URL へのリンク](https://www.amazon.co.jp)
[末尾が閉じ括弧の URL へのリンク](https://ja.wikipedia.org/wiki/マジックナンバー_(プログラム))
[SSLサーバー証明書の検証に失敗する host へのリンク](https://www.tablesgenerator.com/markdown_tables)
[日本語を含む URL へのリンク](https://ja.wikipedia.org/wiki/あ)
user: komagata
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

published_atの記載がないと正しく表示されないケースがあるかもしれません。

published_at: "2022-01-01 00:00:00"
Loading