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

Changed to preserve Surrogate-Key held by ResourceObject at entry point #119

Merged
merged 3 commits into from Jul 25, 2022

Conversation

jingu
Copy link
Member

@jingu jingu commented Jul 19, 2022

  • Pageリソースに直接設定されたSurrogate-Keyを保存するように変更しました。
  • Added surrogate key to Donut view #118 の修正で、ResourceStorageにDonutを保存する際のタグとしては設定できていましたが、Surrogate-Key HTTPヘッダには出力できていない問題を修正しました。

@jingu jingu marked this pull request as ready for review July 19, 2022 10:49
@koriym
Copy link
Member

koriym commented Jul 19, 2022

例えばDonutRepositoryTest::testCreateDonut()テストは以下のようにembedされているサロゲートキーが以下のように含まれます。

image

この時 _html_comment_comment01のキーも含まれますが、この属するドーナッツテンプレートキャッシュの削除は問題にならないでしょうか?

@jingu
Copy link
Member Author

jingu commented Jul 20, 2022

なるほど。再起的な処理なので、一番外側のドーナツの対応をしたつもりが下層のkeyも集めてしまっているという感じですよね?
ただ思うのは、アプリケーションの内部キャッシュとしては _html_comment_comment01 キーがページのキャッシュタグに含まれていなくても良いと思うのですが、CDNが存在する場合はコメントの更新の際にページそのもののキャッシュを消す必要があるので、ページのSurrogate-Keyに _html_comment_comment01 が含まれている必要はあるのでは?とも思いました。

@koriym
Copy link
Member

koriym commented Jul 20, 2022

スペースが小さく勘違いしてましたが、_html_comment_comment01ではなく、_html_commentcomment01でした。(comment01はBlogPostingに含まれるCommentでセットしていました。)

リソースに含まれるサロゲートキーなのでComment01が削除されるドーナツのテンプレートが削除されるとルートのBlogPostingが削除されるのは妥当です。

@koriym
Copy link
Member

koriym commented Jul 20, 2022

含まれるリソースのドーナツテンプレートが変更されたときに、それを含むリソースのドーナツテンプレートを削除されるのは妥当ですが、含まれるリソースの「値」が変更されただけなのにそれを含むリソースのドーナツテンプレートを削除されるのは妥当ではありません。

ここの部分がクリアになっているかに少し不安が残っています。どうでしょうか。

@codecov
Copy link

codecov bot commented Jul 25, 2022

Codecov Report

Merging #119 (2d28fcf) into 1.x (1283e07) will not change coverage.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##                 1.x      #119   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
  Complexity       236       236           
===========================================
  Files             48        48           
  Lines            710       711    +1     
===========================================
+ Hits             710       711    +1     
Impacted Files Coverage Δ
src/DonutRepository.php 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1283e07...2d28fcf. Read the comment docs.

@koriym koriym merged commit 40a0906 into bearsunday:1.x Jul 25, 2022
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