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

KomifloResolverに画像の取得を追加 #30

Merged
merged 4 commits into from Jan 18, 2019

Conversation

eai04191
Copy link
Member

@eai04191 eai04191 commented Jan 14, 2019

未ログインで取得できる情報なので問題ないと考えていますが、問題があるようであればcloseしてください。

画像のサイズについて

Komiflo APIで取得できるサムネイル画像のサイズは

0: "148_desktop_small"
1: "296_desktop_small_2x"
2: "198_desktop_medium"
3: "396_desktop_medium_2x"
4: "247_desktop_large"
5: "494_desktop_large_2x"
6: "160_mobile_narrow"
7: "320_mobile_narrow_2x"
8: "207_mobile_medium"
9: "414_mobile_medium_2x"
10: "188_mobile_large"
11: "376_mobile_large_2x"
12: "564_mobile_large_3x"
13: "346_mobile"

があるようです。

現在Tissueでは .col-md-6 でオカズリンクの .card を表示していて、そのコンテナが .col-lg-8 のため、画像は最大で342pxで表示されるようです。

高DPIな環境を想定して342pxの2倍、684px程度あると良さそうだったので取得できる最も大きなサムネイルの 564_mobile_large_3x を使用しています。

@shibafu528
Copy link
Member

先方のAPI応答内に signature とは別に signature_expires というキーがあるのが気になっています。

MetadataResolverの実装でreturnしている画像URLなどはDBに保存しているため、もし signature が有効期限を持った情報であれば、すぐにリンク切れ状態になってしまうのではないでしょうか。

もしそうなると、DB上のメタデータキャッシュを失効させる仕組みも含めて考えないといけなくなります。

@eai04191
Copy link
Member Author

🦀 🦆 ……ちょっとローカルで様子見てみます。

@eai04191
Copy link
Member Author

eai04191 commented Jan 14, 2019

余談ですが、Komifloには作品の公開期限がありますが、いまのところ公開期限が切れたものもサムネイルの取得は問題なくできそうです。

公開期限が切れている作品例: https://komiflo.com/#!/comics/2132
その作品のAPI: https://api.komiflo.com/content/id/2132

@eai04191
Copy link
Member Author

予想通り signature_expires がすぎるとアクセスが拒否されるようなので先にメタデータに期限をつけるのをやってみます。

@eai04191
Copy link
Member Author

e8c9de3
Carbon::parse に通しました。問題なさそうです。
確認お願いします。

@shibafu528
Copy link
Member

shibafu528 commented Jan 17, 2019

KomifloのAPIが返してくる日時の値はUTC (末尾Z)でCarbonもタイムゾーンを認識しているのですが、どうもこのままORMに投げるとタイムゾーン情報だけ喪失してDBに書き込まれてしまうようです。

DB側カラムが timestamp without time zone 型であることで起きてそうな気はしますが、そこを変えたいとは考えていないのでここはリゾルバ側でサーバ設定のタイムゾーンのローカル時刻まで補正したほうが良いかなと思います。

具体的にはCarbonのインスタンスで setTimezone(config('app.timezone')) を呼んで、Laravelの設定値からタイムゾーンを拾ってセットするとかですかね……。

eai04191 added a commit to eai04191/tissue that referenced this pull request Jan 17, 2019
@shibafu528
Copy link
Member

お手数をおかけしました!コミット整理ありがとうございます!!

@shibafu528 shibafu528 merged commit b6bf1f9 into shikorism:develop Jan 18, 2019
@eai04191 eai04191 deleted the feature-KomifloResolver branch January 18, 2019 17:34
shibafu528 added a commit that referenced this pull request Jan 22, 2019
@shibafu528 shibafu528 mentioned this pull request Jan 22, 2019
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants