Add force ondemand api mode by akiomik · Pull Request #64 · BuffettCode/buffett-code-api-client-google-spreadsheet

akiomik · 2021-12-10T08:21:27Z

shoe116

差分は良さそう。
リリースする前にドキュメント加筆しないと危険だな。

akiomik · 2021-12-11T01:10:38Z

この修正はドキュメントが対応されてからリリースしたい
他の修正は先にリリースする可能性がある

ため、他のリリースをブロックしないようにマージは一旦保留します。

akiomik · 2021-12-13T01:17:14Z

バグ修正は #65 に切り出してrebaseしました。

shoe116 · 2022-01-11T09:35:09Z

+  public set forceOndemandApiEnabled(forceOndemandApiEnabled: boolean) {
+    this._forceOndemandApiEnabled = forceOndemandApiEnabled
+  }


this._ondemandApiEnabled が false だったら exception にしたいな。
逆に、 set ondemandApiEnabled のほうにも、forceOndemandApiEnabled だったら exception 吐くようにしておきたい。

UI でも validation するべきだと思うけど、 Setting class も「自身の辻褄」を守ってほしい

akiomik · 2022-01-14T03:01:36Z

conflictしてたのでrebaseしました。

…abled

akiomik · 2022-01-14T09:22:20Z

いくつかの実装変更を行いました。

be61bb5
- 従量課金モードが有効になっていない場合に強制従量課金モードがオンにできないよう、UIと Setting クラスレベルで整合性を取るように変更
- サイドバーのスタイル変更
0889695
- 将来の拡張性のために、 Setting.forceOndemandApiEnabled というフラグの代わりに Setting.ondemandApiCallMode というステータスを持つように変更

スクリーンショット

以下のシナリオを実行してみています。

初期状態 (従量課金モードと強制従量課金モードが無効) で設定画面を開き、従量課金モードを有効にして保存
従量課金モードのみ有効な状態で設定画面を開き、強制従量課金モードも有効にして保存
従量課金モードと強制従量課金モードが有効な状態で設定画面を開き、従量課金モードを無効にして保存 (強制従量課金モードも無効化される)

2022-01-14.18.17.08.mov

akiomik · 2022-01-14T09:29:58Z

-      result = bcodeDaily(client, ticker, parsedPeriod, propertyName, setting.ondemandApiEnabled)
+      result = bcodeDaily(
+        client,
+        ticker,
+        parsedPeriod,
+        propertyName,
+        setting.ondemandApiEnabled,
+        setting.isOndemandApiCallModeForce()
+      )
    } else {
-      result = bcodeQuarter(client, ticker, parsedPeriod, propertyName, setting.ondemandApiEnabled)
+      result = bcodeQuarter(
+        client,
+        ticker,
+        parsedPeriod,
+        propertyName,
+        setting.ondemandApiEnabled,
+        setting.isOndemandApiCallModeForce()
+      )


BCODE のquarterとdailyそれぞれに雑に setting.isOndemandApiCallModeForce() を渡していますが、将来的には Setting に応じた従量課金の挙動を決定するストラテジークラスみたいなのを実装できると綺麗になりそうです。

akiomik · 2022-01-14T09:31:20Z

  const setting = Setting.load()
  setting.setDefaultToken()
  setting.setDefaultOndemandApiEnabled()
+  setting.setDefaultOndemandApiCallMode()


インストール時に従量課金モードの初期状態を default (5年以上前のデータは従量課金エンドポイントを利用し、それ以外は定額エンドポイント利用する) にします。

これを default って呼ぶかどうかがだいぶ難しいところだけど、まぁ

元々は naive とかにしようとしたけどどのみち伝わらないし、短い言葉で説明するのは限界があるなと思って default としました。結局ただのプレースホルダだしまあなんでもいいかなと。

akiomik · 2022-01-14T09:33:32Z

-    const ondemandQuarterApiPeriods = ondemandQuarterApiPeriodRange.selectOndemandQuarterApiPeriod(ticker, range)
-    const quarterApiPeriods = ondemandQuarterApiPeriodRange.filterOndemandQuarterApiPeriod(ticker, range)
+    let ondemandQuarterApiPeriods = []
+    let quarterApiPeriods = []
+
+    if (setting.isOndemandApiCallModeForce()) {
+      ondemandQuarterApiPeriods = range.range()
+    } else {
+      ondemandQuarterApiPeriods = ondemandQuarterApiPeriodRange.selectOndemandQuarterApiPeriod(ticker, range)
+      quarterApiPeriods = ondemandQuarterApiPeriodRange.filterOndemandQuarterApiPeriod(ticker, range)
+    }


CSVのダウンロード機能は、従量課金モードが default の場合はよしなに従量課金のレンジと定額のレンジに分割して叩き分けますが、従量課金モードが force の場合はすべてのレンジを従量課金として扱います。

akiomik · 2022-01-14T09:35:07Z

-  private constructor(private _token, private _ondemandApiEnabled) {}
+  private constructor(
+    private _token: string,
+    private _ondemandApiEnabled: boolean,
+    private _ondemandApiCallMode: 'default' | 'force'
+  ) {}


もう少し頑張れば ondemandApiCallModes から 'default' | 'force' という直和型を作るようにできそうだったのですが、時間がかかりそうだったので素朴にstring literalで実装してます。

akiomik · 2022-01-14T09:37:22Z

+    if (!this.ondemandApiEnabled && this.ondemandApiCallMode !== Setting.defaultOndemandApiCallMode) {
+      return false
    }


従量課金がオフなのに従量課金モードが default じゃない場合はinvalidとして扱っています。

akiomik · 2022-01-14T09:40:18Z

+      input[disabled] + label {
+        text-color: #777;
+      }


disabled なinput要素に隣り合う label 要素をグレーアウトさせるcssです。

akiomik · 2022-01-14T09:42:16Z

+          <li>
+            <input id="force-ondemand-api-check" type="checkbox" disabled="disabled">
+            <label for="force-ondemand-api-check"><b>常に従量課金APIを利用する (制限回避モード)</b></label>
+          </li>


設定の読み込みが非同期なので、初期状態では強制従量課金モードは disabled にしています。設定読み込みが完了したときに従量課金が有効になっていたら disabled を解除しています。

akiomik · 2022-01-14T09:45:57Z

+      function onOndemandApiCheckChange() {
+        const ondemandApiEnabled = document.getElementById('ondemand-api-check').checked
+        const forceOndemandApiCheck = document.getElementById('force-ondemand-api-check')
+
+        // ondemand-api-checkの変更の都度初期化する
+        forceOndemandApiCheck.checked = false;
+
+        if (ondemandApiEnabled) {
+          forceOndemandApiCheck.disabled = false;
+        } else {
+          forceOndemandApiCheck.disabled = true;
+        }
      }


従量課金を有効にするチェックボックスのオンオフが切り替わったときに呼ばれる関数です。
状態が変わったらその都度強制従量課金モードはオフにしています。また、従量課金が有効になっている場合のみ、従量課金モードのチェックボックスを触れるようにしています。

akiomik · 2022-01-14T09:47:26Z

 export function loadSetting(): object {
-  return Setting.load().toObject()
+  const setting = Setting.load()
+  const obj = setting.toObject()
+  obj['forceOndemandApiEnabled'] = setting.isOndemandApiCallModeForce()
+  return obj
 }

-export function saveSetting(token: string, ondemandApiEnabled: boolean): void {
+export function saveSetting(token: string, ondemandApiEnabled: boolean, forceOndemandApiEnabled: boolean): void {
  const setting = Setting.load()
  setting.token = token
  setting.ondemandApiEnabled = ondemandApiEnabled
+  setting.ondemandApiCallMode = forceOndemandApiEnabled ? 'force' : 'default'
  setting.save()
 }


UI上は「強制従量課金モードが有効かどうか」というbooleanで持ち、アプリ内での実データは「従量課金モードがどれか」という持ち方を行っているため、そのデータをUIからの設定読み出し時に相互に変換しています。

akiomik · 2022-01-14T09:48:28Z

  "compilerOptions": {
    "sourceMap": true,
    "target": "es5",
+    "lib": ["es2017"],


Object.values や Array.includes が使えないのが不便だったのでes2017形式で書けるように変更してます。出力はes5形式のままです。

shoe116

例外処理のところだけ、好みですが気になりました

shoe116 · 2022-01-14T09:55:56Z

  const setting = Setting.load()
  setting.setDefaultToken()
  setting.setDefaultOndemandApiEnabled()
+  setting.setDefaultOndemandApiCallMode()


これを default って呼ぶかどうかがだいぶ難しいところだけど、まぁ

akiomik force-pushed the add-force-ondemand-api-mode branch 3 times, most recently from 1060529 to 0cb33ab Compare December 10, 2021 09:53

akiomik commented Dec 10, 2021

View reviewed changes

Comment thread src/api/v3/client.ts Outdated

shoe116 reviewed Dec 11, 2021

View reviewed changes

akiomik changed the title ~~Add force ondemand api mode~~ [DO NOT MERGE] Add force ondemand api mode Dec 11, 2021

akiomik self-assigned this Dec 11, 2021

akiomik force-pushed the add-force-ondemand-api-mode branch from 1800fcb to f1e82e9 Compare December 13, 2021 01:16

shoe116 reviewed Jan 11, 2022

View reviewed changes

Add force ondemand api mode

324d65a

akiomik force-pushed the add-force-ondemand-api-mode branch from f1e82e9 to 324d65a Compare January 14, 2022 02:12

akiomik added 2 commits January 14, 2022 18:15

Add forceOndemandApiEnabled validation

be61bb5

Add Setting.ondemandApiCallMode instead of Setting.forceOndemandApiEn…

0889695

…abled

akiomik commented Jan 14, 2022

View reviewed changes

Comment thread src/setting.ts

akiomik commented Jan 14, 2022

View reviewed changes

akiomik changed the title ~~[DO NOT MERGE] Add force ondemand api mode~~ Add force ondemand api mode Jan 14, 2022

shoe116 reviewed Jan 14, 2022

View reviewed changes

Change fallback code of Setting.castOndemandApiCallModeString

0574846

shoe116 approved these changes Jan 14, 2022

View reviewed changes

akiomik merged commit 0a33634 into development Jan 14, 2022

akiomik deleted the add-force-ondemand-api-mode branch January 14, 2022 10:18

akiomik mentioned this pull request Jan 14, 2022

Release v14 #81

Merged

Conversation

akiomik commented Dec 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

shoe116 left a comment

Choose a reason for hiding this comment

Uh oh!

akiomik commented Dec 11, 2021

Uh oh!

akiomik commented Dec 13, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akiomik commented Jan 14, 2022

Uh oh!

akiomik commented Jan 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

スクリーンショット

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akiomik Jan 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akiomik Jan 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

akiomik Jan 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shoe116 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

akiomik commented Dec 10, 2021 •

edited

Loading

akiomik commented Jan 14, 2022 •

edited

Loading

akiomik Jan 14, 2022 •

edited

Loading

akiomik Jan 14, 2022 •

edited

Loading

akiomik Jan 14, 2022 •

edited

Loading