add user-agent bot in ParseMetaForURL #1956
Conversation
There was a problem hiding this comment.
あとは
traQ/service/ogp/parser/domain.go
Line 19 in 8a2db60
edit: ごめん、twitterの特別分岐はいらなくなったんでしたね。上のテストのSkipNowはそのままで良いです。
実際にリクエストを外部サイトに送るテストは賛否両論あるかもしれませんが、
https://github.com/traPtitech/traQ/blob/8a2db60a416f32cf022a74c4bd795d6a4c468b84/service/ogp/parser/domain_twitter_test.go
と似た内容のテストを書けると、動作確認が他の人/あとからでもしやすいかもしれないです。
service/ogp/parser/parser.go
Outdated
| return nil, nil, ErrNetwork | ||
| } | ||
|
|
||
| req.Header.Add("user-agent", `bot`) |
There was a problem hiding this comment.
user-agentはアクセスしたものが何者なのかを相手に知らせる目的もあるので、可能なら既にあるこちらがわと合わせられると嬉しいですね
traQ/service/ogp/parser/domain_vrchat.go
Line 57 in 8a2db60
There was a problem hiding this comment.
書き直さなくても、同じパッケージなので単純に変数を参照できます
| req.Header.Add("user-agent", `bot`) | |
| req.Header.Add("User-Agent", userAgent) |
There was a problem hiding this comment.
Twitterがbotという文字列が入っていないとOGPを返さないようなので、少し名前を変えました
|
テスト書いたの初めてなので、変なところあると思います。指摘お願いします。 |
| @@ -122,3 +125,39 @@ func TestExtractTitleFromNode(t *testing.T) { | |||
| assert.Equal(t, "", result) | |||
| }) | |||
| } | |||
|
|
|||
| func TestFetchTwitterOGP(t *testing.T) { | |||
| tests := []struct { | |||
There was a problem hiding this comment.
他のところにもついているし、並行化しても問題ないと思うので、t.Parallel()付けていいと思います。もし意図があれば別です。
There was a problem hiding this comment.
Parallelにするなら、ループ変数のttをキャプチャしないように気をつけないといけないです
(ループの中にtt := ttを追加)
1.22とかで変更されそうな仕様だけど
There was a problem hiding this comment.
あ、僕が意図してたのはループの中じゃなくて関数の初めです。ループにも付けていいと思います。
There was a problem hiding this comment.
両方とも平行化できたと思います。e2ee810
|
あるので、マージします。 |
TwitterのOGPが表示されないのを解消する目的です。
手元で試したところ投稿やページ、ユーザーによっては表示されないものもあるようです。
ついでにカクヨムのOGPも表示されるようになったようです。
Twitter.com(x.com)のみに絞るべきか悩んだのですが、上のカクヨムの例があったので、一旦OGPのために飛ばすGETすべてに
user-agent:botをつけています。