-
-
Notifications
You must be signed in to change notification settings - Fork 7
多言語対応のLocale追加やページ設定でサイト名を上書きできるようにしました。 #2306
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
Conversation
masaton0216
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
お疲れ様です。
ご対応ありがとうございます。
デバッグのトコ、あまり重要ではないのですが、クエリの部分がパフォーマンス観点で気になっています。
| // 言語ルートのデバッグ表示 | ||
| //echo $language_root . "<br />"; | ||
| //echo strpos($page['permanent_link'], '/', 1) . "<br />"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
デバッグコードとして必要であればLog::debug()に置き換えるのはどうでしょう?
| // 言語ルートをキーにして多言語設定を絞り込み | ||
| $configs = $request->attributes->get('configs'); | ||
| $configs_multilanguage = $configs->where('category', 'language')->where('additional1', $language_root); | ||
| //print_r($configs_multilanguage); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
デバッグコードとして必要であればLog::debug()に置き換えるのはどうでしょう?
同上です。
| $alternates[str_replace('/', '', $page->additional1)] = $page->permanent_link; | ||
| } | ||
| $request->attributes->add(['alternates' => $alternates]); | ||
| //print_r($alternates); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
デバッグコードとして必要であればLog::debug()に置き換えるのはどうでしょう?
同上です。
| $multilanguage_pages = Page::join('configs', function ($join) use($no_lang_path) { | ||
| $join->on('configs.category', '=', DB::raw("'language'")) | ||
| ->on('pages.permanent_link', 'LIKE', DB::raw("CONCAT(configs.additional1, '" . $no_lang_path . "')" )); | ||
| }) | ||
| ->select('pages.*', 'configs.additional1') | ||
| ->get(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
懸念
- LIKE検索にconcat()組み合わせるとインデックスが効かない(フルスキャンになる)
- 言語設定数 * ページ数の組み合わせで走査範囲が広がる
- 当該処理は毎リクエスト実行される
という観点から、パフォーマンス懸念があります。
JOINを廃止して、WHERE句での完全一致検索に変更する案はどうでしょう?
※但し、言語数分だけクエリが実行されます。
(通常は数十もの言語を1つのサイトに設定することはない(2~せいぜい5ぐらい?)という想定が前提です。これに限らない場合はもう少し要件踏まえて調整してもいいかもしれません。)
調整コード
// Configsテーブルから全言語設定を取得 ※後で文字列結合しやすくする
$language_roots = $configs->where('category', 'language')
->pluck('additional1') // additional1カラムのみ抽出 ※イメージ)['/', '/en-GB', '/es-ES']
->map(function($root) { // 前後の "/" を除去 ※イメージ)['', 'en-GB', 'es-ES']
return trim($root, '/');
})
->filter() // 空文字を除外 ※イメージ)['en-GB', 'es-ES']
->values() // インデックスを振り直し ※イメージ)[0 => 'en-GB', 1 => 'es-ES']
->toArray(); // 配列に変換 ※イメージ)['en-GB', 'es-ES']
if (empty($language_roots)) {
return $next($request);
}
// 検索結果を格納するコレクション
$multilanguage_pages = collect();
// 各言語ルートに対してループ
foreach ($language_roots as $root) {
// 検索対象のパスを構築
$search_path = '/' . $root . $no_lang_path;
// 完全一致検索(インデックスが効く)
$page_found = Page::where('permanent_link', $search_path)->first();
if ($page_found) {
// 言語ルート情報を追加
$page_found->language_root = '/' . $root;
// コレクションに追加
$multilanguage_pages->push($page_found);
}
}
// 多言語のページ一覧を画面に渡すためにリクエストに設定
$alternates = [];
foreach($multilanguage_pages as $page_obj) {
$locale = trim($page_obj->language_root, '/');
if (empty($locale)) {
// デフォルト言語の場合、設定から取得
$default_locale = $configs->where('category', 'language')
->where('additional1', '/')
->first();
$locale = $default_locale ? $default_locale->value : 'ja';
}
$alternates[$locale] = $page_obj->permanent_link;
}
$request->attributes->add(['alternates' => $alternates]);
return $next($request);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
サンキューです。
LIKEやめたいですね。
設定言語数が多いときにクエリをたくさん発行するのも微妙。とかも思うので、見直したところ、
wherein で取得できるかなと思えてきたので、見直してみます。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
修正してみました。
LIKEはやめて、whereInで指定できるように考えました。
これで、LIKEによるSQLクエリと、言語数ループする処理のパフォーマンス問題はクリアできたかなと思います。
ついでに処理全体もわかりやすくしようと思って修正しています。
masaton0216
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ご対応ありがとうございます。
もう少しだけお付き合いください。
| // \Log::debug(strpos($page['permanent_link'], '/', 1)); | ||
|
|
||
| // 言語ルートをキーにして多言語設定を絞り込み(現在のページが多言語設定の対象か判断するため) | ||
| $configs = $request->attributes->get('configs'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
34行目と重複しているので、こちらの行は削除してよさそうです。
| // \Log::debug(print_r($multilanguage_ins, true)); | ||
|
|
||
| // Pageテーブルから現ページと同じ意味を持つ多言語ページのpermanent_linkを取得する。 | ||
| $multilanguage_pages = Page::select('pages.*') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pagesテーブルは行列それなりに多いテーブルなので、取得する列数絞るとデータ転送&メモリ量は減らせそうです。
$multilanguage_pages = Page::select('permanent_link')
| // 言語ルートをキーにして多言語設定を絞り込み(現在のページが多言語設定の対象か判断するため) | ||
| $configs = $request->attributes->get('configs'); | ||
| $configs_multilanguage = $configs->where('category', 'language')->where('additional1', $language_root); | ||
| // \Log::debug(print_r($configs_multilanguage, true)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
伝え方が曖昧ですいません。
意図はコメントアウト外してデバッグコードとして残す、という意味です。
結論コードは下記です。.envでdebugレベルを変更すれば適宜、.logファイルに出力されます。
\Log::debug($configs_multilanguage);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
失礼。
バグがあった際などは、改めて考えると思うので、このデバッグコードはそこまで残しておくものではないです。
ということで、今回のデバッグコードは削除しますね。
|
|
||
| // 設定されている各言語のルートパスをconfigから絞り込む | ||
| $configs_multilanguage_roots = $configs->where('category', 'language'); | ||
| // \Log::debug(print_r($configs_multilanguage_roots, true)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
意図はコメントアウト外してデバッグコードとして残す、という意味です。
同上です。
| // '//'の変換は、configの多言語設定URLでトレイリングスラッシュのアリ/ナシをどちらも許容しているため。 | ||
| $multilanguage_ins[str_replace('/', '', $configs_multilanguage_root->additional1)] = str_replace('//', '/', $multilanguage_in); | ||
| } | ||
| // \Log::debug(print_r($multilanguage_ins, true)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
意図はコメントアウト外してデバッグコードとして残す、という意味です。
同上です。
| $alternates[$this->getLocale2Path($page->permanent_link)] = $page->permanent_link; | ||
| } | ||
| $request->attributes->add(['alternates' => $alternates]); | ||
| // \Log::debug(print_r($alternates, true)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
意図はコメントアウト外してデバッグコードとして残す、という意味です。
同上です。
|
こちらこそ、付き合っていただき、感謝感謝です。 |
|
修正しましたー。 |
|
対応お疲れ様でした! |
詳しくは \app\Enums\ConnectLocale.php を確認してください。
概要
同じサイトに他の言語でのページを作りたかったため。
以下のサイトが参考になると思います。
https://tatsumi-shinonome.jp/multi-lunguage
レビュー完了希望日
関連Pull requests/Issues
opensource-workshop/connect-cms-ideas#180
参考
DB変更の有無
有り
チェックリスト