ODCのAI Mentor System (AIMS) のパターンを見ていく（2024/10時点）1/2 #outsystems - Qiita
の続き。
AIMSで指摘事項として挙がるパターンについて、ドキュメントを一通り確認し、コメントしていく。
対象は2024/10/5時点のパターンを解説する公式のドキュメント。
性能カテゴリのパターン
Inefficient empty list test (SQL・Aggregate結果が空であることを確認するのに、Countを使っている）
Inefficient query countと重複している気もするが、SQL・Aggregateの結果Listが空であることを確認するには、.Emptyを使えば良い。
Avoid long-running Timers (TimerのTimeoutが30分を超えている）
TimerのTimeoutはデフォルト20分、最長60分。
これにさらに30分を超えるとAIMSの指摘に引っかかると言う制約が追加されることになる。
では、実際にこれを上回る実行時間が必要な場合どうするかというと、O11の頃からあるベストプラクティスに基づき、
- Timeoutより短い論理的な上限時間をローカル変数やSettingsで持つ
- ループ中でこの時間にかかったらそこまでのトランザクションをコミット
- 同じTimerをWakeして現在の実行を終了する
というパターンを取る。
CSS in the screen's style sheet (ScreenレベルでCSSが定義されている）
Inline CSS styleと同じ種類の問題。あっちは、個別のUI要素への設定。こっちは、Screen単位のCSS設定をした場合の指摘。
Screen限定のCSSはScreenレベルのStyle Sheetに定義したくなるが、ThemeにClassを定義して、それをScreen内で利用することが推奨とのこと。
Mobileの場合はちらつきが出るそうなので仕方ないが、Webはそんなに気にする必要があるかは疑問がある。開発者は、CSSにそんなに気を使ってくれないと思うので、Screen専用のスタイルをThemeで定義するとTheme内のあちこちに同じScreen用の定義が分散してしまいそう。
Large resource (Resourcesに格納するファイルサイズがモバイル 150KB、Web 500KBを超えている）
ファイルサイズが大きくなることによって、ダウンロードとPublishにかかる時間が長くなる。
Reduce the size of the resources to the minimum needed for their usage (below 150KB/500KB for Mobile/Web Applications).
という記述があるので、恐らく、「Resoucesにあるファイルすべての合計」がこのサイズを超えると問題があると思われる。
かなり小さい。これだと、Excel帳票のテンプレートを格納するのも結構厳しそう。
Large image (サイズがモバイル 150KB、Web 500KB、解像度が1024pxを超える画像がApp内にあるを超えている
↑のと同じく、Appのダウンロード・Publish速度を気にする指摘。
OML以外の場所に置けば良い。
Non-optimized local data fetch （Mobileのみ。On Initialize/On Ready/On Render内から local data fetchをしている）
local data fetchが何を指すのかあまり明確ではないが、恐らくLocal Entityの取得だろうか。
これらのイベント内では処理を同期的に待ってしまうから、非同期の並列実行の恩恵を受けられないためとのこと。
Non-optimized Local Storage (Mobileのみ。クライアントのEntityがサーバー側Entityのコピーであるか、Local Storageが複雑すぎる）
Local Storageが正確に何を指すのか（LocalのEntityか、それを使ったAggregateも指すのか）が文脈上明らかではないが、Mobileであっても、あくまでサーバー側を正とし、クライアント側は必要最小限にするべきという考えらしい。
Not taking advantage of Local Storage（Mobileのみ。ScreenにData Actionが多すぎる）
Data Actionはサーバーからデータを取得する。これは通信を伴うので、可能なものは、事前に取得してLocalのEntityに持っておくべき、という考えのようだ。
閾値が示されていないが、もしData Actionが5個、6個とぶら下がっているようならたしかに多い。
Incorrect offline sync method (Mobileのみ。OfflineDataSync等の標準の同期の仕組みを使っていない）
Mobileアプリを作るとOfflineDataSync等が自動で作成される。恐らくサーバー側のEntityとクライアント側のEntityの同期にこれを使っていないということか。次の項目との差分が明確ではない。
こういう仕組みは標準化したほうがいいのは確かだが。実際に指摘されたケースを見て検討してみたいところ。
No asynchronous Offline sync (Mobileのみ。サーバー側Entityからクライアント側Entityへの同期をUIのロジックやActionで同期的に行っている）
↑の項目と同じく、自動作成される同期の仕組みの中に、同期処理を非同期にキックするものがあるのでそれを使うべき、という指摘と思われる。
Complex splash screen (Mobileのみ。Splash Screenにサーバー呼び出し、複雑なロジック、多量のBlockが配置されている）
Splash Screenは、Mobileアプリを起動するときに最初に表示されるシンプルな画面のこと。
アプリの準備が整うまで表示しておいたりする。この用途なので、シンプルに保つ必要があるのはその通り。
Server requests in client events (Screen/BlockのLifecycle Eventでサーバー処理を呼んでいる）
Lifecycle EventはOn Initialize/On Ready/On Render/On After Fetchのこと。これらのイベントハンドラ内ではその実行を待つのでパフォーマンスが落ちるとのこと。
ちょっと気になるのは、Screen下のActionにサーバー呼び出しを切り出し、そのActionをこれらのイベントで非同期に呼び出したときに指摘に挙がるか、これは後で試してみる。
セキュリティカテゴリのパターン
Avoid setting screens as accessible by everyone (Screenを認証を要求しない設定にしている）
ScreenのプロパティAuthorization > Accessible byをEveryoneにしていると発生する。
この設定は認証を要求しないので、誰でもその画面にアクセスできる。
ログイン画面や、エラー表示用画面ではEveryoneにすることはやむを得ないことがある（特にログイン画面）。
ログイン画面の場合、名前をLoginにするとこの指摘に引っかからないようだが、こういう名前のリストは公開してほしいところ。企業ごとに設定できるとなお良い。
Exposed REST services without authentication（Expose REST APIでSecurity > AuthenticationがNoneに設定されている）
ODCでは、ユーザー名（＋パスワード）を指定してログインさせる方法が無い (OIDCのフローを通ることになる) 。そのため、Expose REST APIで認証を実装し、認証したユーザーの情報を後続で使おうと思うと、OnAuthenticationコールバックと各REST API Methodで二重実装になってしまうが、一応Authenticationは設定しておいた方が良い。Customになっているからといって、認証処理が実装されているとは限らないが、そもそも認証を意識していなかったというケースは抽出できる。
Insecure usage of GetUserId in client Block parameters (BlockのInput ParameterにGetUserId()の結果を渡している）
クライアントサイドではユーザーが（悪意と知識があれば）色々操作できるので、セキュアに保つべきものはすべてパラメータで持ち回らない方が良い（サーバーサイドで認証情報から取得するようにする）。
Blockに限らずクライアントサイドから渡された認証情報をサーバーサイドのパラメータに使う実装は意外と見るので、なるべく早めにレビュー等で是正しておきたい（あちこちで使われてしまったあとだと直すのが難しくなるので）。
Insecure usage of GetUserId function on client context (クライアント側で取得したGetUserId()の値をサーバー側のパラメータにしている）
↑と同じ理由で危険。また、レビューしていると本当にこういう実装をしているのを見る。あとから直すのは大変なので、AIMSで自動指摘してくれて、かつ、AIMSの指摘を全部潰すのを日課にしてもらえれば、だいぶ楽になる。
Screen Aggregates exposing system entities accessible by anyone (Authorization > Accessible byがEveryoneに設定されているScreenで(System)のEntityにアクセスしている）
今のところ(System)の下のEntityはUserしかないが、EmailとNameを含んでいるので気はつけた方が良い。
そもそもEveryoneがアクセスできるScreenが別のパターンの指摘対象だが、恐らく今後(System)のEntityは増えていくので、別の指摘として存在することには恐らく意味が出てくる。
SQL injection (SQLのQuery ParameterのExpand InlineプロパティをYesにしている）
SQL文の一部をエスケープせずに、Query Parameter経由で渡せるオプションだが、SQL Injectionの懸念があるので指摘として挙がる。
実際に必要で、SQLインジェクションを防げることを実装上で確認できたらDismissedにする。SQLを確実に判断できる人がプロジェクトにいれば、だが。
Visible disabled Button (Enabled=FalseにしたButtonがVisible=Trueのまま残っている）
Enabled=Falseにしたところで、HTMLにタグは出力される。これを開発者ツール等で変更することでイベントハンドラーを起こせる。そのため、Enabled=Falseにしただけでイベントハンドラーが動作することはないと考えてしまうのを防ぐためのパターンと思われる。
なにか理由があり、サーバ側でセキュリティを担保しているならDismissedでも良さそう。