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

fix: Error when creating pages with deep hierarchy #9487

Merged

Conversation

reiji-h
Copy link
Contributor

@reiji-h reiji-h commented Dec 13, 2024

Summary

  • 深い階層のページを作成する際にエラーが出ないように

Task

Noes

  • 動作確認をしている中で、510 程度のスラッシュの数を持つパスでエラーが発生したので、500 以上は作れないようにしている。

    • だがそもそも 500 も階層があるページを作成する際は動作が重くなり現実的に作成できないことが多い。
  • 深い階層のページを作成する方法は次の2つを想定しており、どちらでも作成できない旨を伝えられることを確認済み

    • ページツリーから作成する
    • permalink で作成

スラッシュ 5 個以上は作れないようにして実験

スクリーンショット 2024-12-13 165630
スクリーンショット 2024-12-13 165705

Copy link

changeset-bot bot commented Dec 13, 2024

⚠️ No Changeset found

Latest commit: 15f416c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@miya miya requested review from yuki-takei and miya and removed request for yuki-takei December 16, 2024 10:37
@@ -117,6 +117,7 @@ const restrictedPatternsToCreate: Array<RegExp> = [
/^\/(_search|_private-legacy-pages)(\/.*|$)/,
/^\/(installer|register|login|logout|admin|me|files|trash|paste|comments|tags|share|attachment)(\/.*|$)/,
/^\/user(?:\/[^/]+)?$/, // https://regex101.com/r/9Eh2S1/1
/^(\/.+){500,}$/, // avoid deep layer path. see: https://regex101.com/r/L0kzOD/1
Copy link
Member

Choose a reason for hiding this comment

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

500も一般的な利用の範囲では現実的ではないので、130にしましょう

考え方

ユーザーの潜在ニーズとして、MongoDB 内の全ページを一般的な OS のファイルシステム上にディレクトリツリーとして再構築できるのがのぞましい。その際の最大階層はいくつか?

参考

image

image

裏取り

https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#maximum-path-length-limitation

https://developer.apple.com/library/archive/documentation/FileManagement/Conceptual/FileSystemProgrammingGuide/FileSystemOverview/FileSystemOverview.html#//apple_ref/doc/uid/TP40010672-CH2-SW1

Apple の方は

You should assume that the number of directories in a path is limited to 100 or fewer.

のような記述は見当たらず…
Claude を問い詰めたら「質問の根拠となる情報を適切に示せず、誤った情報を述べてしまい大変失礼いたしました。」と謝られた。

だが Win に 260 char 制限があるのは確かなので、今回は130とする。

@yuki-takei yuki-takei removed the request for review from miya December 17, 2024 17:31
mergify bot added a commit that referenced this pull request Dec 17, 2024
@mergify mergify bot merged commit b239b3d into master Dec 17, 2024
22 checks passed
@mergify mergify bot deleted the fix/156799-156858-stop-growi-when-someone-create-deep-page branch December 17, 2024 17:46
@github-actions github-actions bot mentioned this pull request Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants