Skip to content
This repository has been archived by the owner on Jun 1, 2023. It is now read-only.

非機能要件(可読性・拡張性・設計) 問題点・改善点の整理 #2790

Open
nard-tech opened this issue Apr 3, 2020 · 37 comments
Labels
discussion 議論が目的、または議論中の Issue improvement 改善や新機能の要望

Comments

@nard-tech
Copy link
Contributor

nard-tech commented Apr 3, 2020

改善詳細 / Details of Improvement

  • 感染者の増加が続く現状を見ると,このサイトが必要とされる状況はしばらく続くと思われる
  • 一方,一時期に比べると issue, PR の数,merge の頻度は落ち着いており,機能を次々に追加していく,バグを直していくというフェーズから抜けつつある (cf. https://github.com/tokyo-metropolitan-gov/covid19/graphs/code-frequency)
  • したがって,今までの大勢による活発な開発によりやむを得ず生じてしまった「技術的負債」の返済をしつつ,より堅牢で拡張性の高い設計になるよう,非機能要件の問題・改善すべき点を整理し,適切な順序・タイミングでリファクタリングを進めていく必要があると考える

という訳で,この issue に非機能要件の問題・改善すべき点を,必要性やデメリットも含め,まとめていきつつ議論を進めたいと思います.

@nard-tech nard-tech added the improvement 改善や新機能の要望 label Apr 3, 2020
@nard-tech
Copy link
Contributor Author

nard-tech commented Apr 3, 2020

component のディレクトリ構成(案)

  • page/index.vue のみで使われている components/ 以下のファイルを,components/index 以下に移動する(components/flow と同様)
    • components/cards/ 以下のファイルも,components/index/cards/ に移動する
    • components/cards/import している component(主にグラフ描画用の XxxxChart)を components/index/cards/charts/ に移動する
      • (2020-08-22 追記XxxxCard に対応する component は XxxxChart という命名に統一する
    • DataTableComfirmedCasesAttributesTable に,TimeStackedBarChartTestedNumberBarChart に名前を変更すべきでは?
  • page/flow.vue のみで使われている components/ 以下のファイルを,components/flow 以下に移動する(PrinterButton のみ?)
  • (2020-08-22 追記ConfirmedCasesIncreaseRatioByWeekChart.vue は使われていないので削除すべき

@nard-tech

This comment has been minimized.

@nard-tech
Copy link
Contributor Author

nard-tech commented Apr 3, 2020

component の中身が全体的に荒れている感あり

その他,page/index.vue 上の component, chart が全体的に荒れている印象があるので,dry で統一感のある構成にしたい.

@nard-tech
Copy link
Contributor Author

nard-tech commented Apr 3, 2020

JSONファイルとVueの間に中間層を設ける

data/data.json などからデータを取得する際に,中間層を設けることで型推論を可能とし、コードの保守性を上げる.

cf. JSONファイルとVueの間に中間層を設ける (#1100)

実装すると宣言した方がいらっしゃるが,音沙汰なしの状況.

@nard-tech

This comment has been minimized.

@nard-tech
Copy link
Contributor Author

nard-tech commented Apr 3, 2020

data.json について

分割すべきでは?

  • 現在 7727 行と肥大化している.将来的にデータが溜まっていくとさらに肥大化が進み,管理しにくくなると予想される.
  • 他県版に東京版のコードを merge しようとすると conflict が多発する.data.json を分割しなくても conflict することには変わりないが,conflict 解消時のミスの危険性が増すと思われる.
  • 各グラフのデータの更新タイミングが異なるので,data.json を分割した方がファイルごとの責務が明確になり,Git で管理するときに差分が確認しやすい.
  • ただ,tool/convert.php の書き換えも含め,工数が大きく,データの信頼性を担保したまま分割するのは大変.

property の日本語を英語にすべきでは?

  • 個人的な意見ではあるが,property に日本語を使用しているために日本以外からの PR,日本以外への fork が難しい状況になっており,勿体ないと感じる.

@nard-tech

This comment has been minimized.

@khsacc
Copy link

khsacc commented Apr 3, 2020

非機能要件ということで、各vueファイルの<script>の言語がjs, tsで統一されていないのが少し気になっているのですが、どうお考えでしょうか……?
個人的にはすべてtsに書き換えたい気がします。

@mikkame
Copy link
Contributor

mikkame commented Apr 3, 2020

tool/convert.php について

すみません、これについてはすでに保守されておらず・・・。
アクセス権を含む(一応Secretで管理しています)ので万一漏れてしまう事を考え別途管理に移行しています。
現在のtoolについては参考までにという形で残す(READMEにその旨記載)か削除で良いかと思います。
本来はオープンソースで管理したいところです・・・。

@nard-tech
Copy link
Contributor Author

@khsacc

非機能要件ということで、各vueファイルの<script>の言語がjs, tsで統一されていないのが少し気になっているのですが、どうお考えでしょうか……?
個人的にはすべてtsに書き換えたい気がします。

<script> を機械的に <script lang="ts"> に置き換え,型情報が足りず警告が出るような箇所には型を追加する,というのであれば手早く対応できると思います.

@mikkame
Copy link
Contributor

mikkame commented Apr 3, 2020

data.json について
分割すべきでは?

この件についてはその通りだと思います。
カードごとのデータにした方が良いかと思います。(metroやnewsのように)
単純に1階層掘って分離してしまえばあまり影響は大きくないのかなというところと
data.jsonと平行して管理して随時変更してもいいのかなとも思います。
がしかし、他のプロジェクト(本プロジェクトは全く関係ない)がdata.jsonを引っ張って使っている可能性があるので今の時点で急にdata.jsonのサポートを切ってもいいのかという疑問も残ります。
外部サイトのことを気にしすぎることもないとは思いますが・・・

@nard-tech
Copy link
Contributor Author

@mikkame

他のプロジェクト(本プロジェクトは全く関係ない)がdata.jsonを引っ張って使っている可能性
それについては Slack でアナウンスして,数日後にサポートを切るということにすれば十分だと思います.それ以上は面倒見切れないですよね…

tool/convert.php を見たところ,Excel ファイルをパースして JSON を作っていますが,それは別途管理している現在も変わらないでしょうか?
data.json の中にはどこからも使われていない無駄なデータもある気がしますので,そういうものを消していく意味でも,あるいは他のデータを追加するときに誰でも手早く変換スクリプトが書けるようにするためにも,セキュリティが絡むところと絡まないところは適切に分離して,安全な部分については GitHub で管理して頂けるとよろしいかと思います.

@mikkame
Copy link
Contributor

mikkame commented Apr 3, 2020

@nard-tech はい、その流れ自体は変わりません。
またその上で所定の場所からエクセルをダウンロードする機能が追加されております。(アクセス権部分)
その元のエクセルを公開することが難しいという事ですので分離するにもなかなか厳しい状態です。
ご理解いただけると助かります。

@mcdmaster
Copy link
Contributor

PHP プログラムは、JS or TS に書き換えましょう。何でしたら、私がやります(笑) Excel オブジェクトとの I/F がありますので、TS がいいんでしょうね(適当

あと、セキュリティ観点だと以下のような問題があります。
・言語ごとにメンテナンスのノウハウが異なる場合、リソースがより多く必要となることが想定される。そしてそれはコミュニケーションチャネルの予期せぬ増加や追加コストの発生といった問題を招く
・言語ごとの実行エンジンのバージョン・ライフサイクルが異なる場合、アップグレードの度にサービスを停止せねばならないとしたら、エンドユーザがサービス停止の不利益を被るタイミングが単一プラットフォームの場合よりも増える
・1個のシステムに複数の言語が混在している場合、一貫性(integrity)を欠き、それが脆弱性の源となり得る

あくまでガバナンス、コンプライアンス、セキュリティなどを論じる際にありがちな一般的な話になってしまいました。正直、この状態では意見としてまだまだ浅いと思います。
ぜひ、エキスパートの皆さんのご意見を頂戴したく

@mikkame
Copy link
Contributor

mikkame commented Apr 4, 2020

@maccostar 当時はとりあえずデータが作れなければならないということだったので得意言語で実装してしまった経緯があります、ですので得意な方TS/JSに書き換えていただけると助かります。
ただ、上記に記載したとおりの課題がございますのですぐに書き換えるということが難しいです。
何かアイデアが有りましたらよろしくお願いします

@nard-tech
Copy link
Contributor Author

nard-tech commented Apr 4, 2020

@mikkame

所定の場所からエクセルをダウンロードする機能が追加されております。(アクセス権部分)
その元のエクセルを公開することが難しい

一般的な設計論の話になりますが,ファイルを

  • (1) 任意の URL にあるファイルをダウンロードする機能
  • (2) (1) を利用して(公開できない Excel ファイルの URL を指定して)Excel ファイルをダウンロードする機能
  • (3) (2) でダウンロードした Excel ファイルをパースして JSON に展開する機能

に分離して,(1) だけ(Excel ファイルの表の構成が推測されても構わないのであれば (3) も GitHub で公開すれば済むと思います.

現在の非公開のダウンロード,パースのスクリプトも PHP で書かれているのでしょうか?

@nard-tech
Copy link
Contributor Author

@halsk こちらの issue,discussion タグを付けてください.よろしくお願いします.

@mikkame mikkame added the discussion 議論が目的、または議論中の Issue label Apr 4, 2020
@mikkame
Copy link
Contributor

mikkame commented Apr 4, 2020

@nard-tech お返事ありがとうございます。

現在の非公開のダウンロード,パースのスクリプトも PHP で書かれているのでしょうか

生憎ですがその通りです。

(1)の部分ですが、SharePointのAPIを経由してファイルをダウンロードする機能が実装されております。(実装上はOAuthなんですがSharePointAPIが特殊で自前で書き直した経緯があります)
(3)に関しては構造を公開していいのか確認しています。公開できるようならExcelごと置いてしまえるので一番かと思います。
またはダミーExcelとか、今のExcelから出せる項目だけのCSV?を出力するところまでやって
適切な変換をしてもらえるような形にするのも良いかと思います(アイデアレベルです)。
この件については再度確認中ですのでもうしばらくお待ちください。

@mcdmaster
Copy link
Contributor

ということで、イキナリ #1471 からメンションしました。論点は、プライバシーの扱いについてです。
本プロジェクトの成果である東京都 COVID-19 情報サイトは、東京という都市の世界の中の立ち位置からすれば、世界中の人たちが関心を持って見ていると考えられます。
世界の中でも欧州からのアクセスがあった場合、GRPR という規定を守ることが求められます。これは、欧州拠点の発信者だけでなく、域外にも適用されるものです。
COVID-19 情報サイトでは、サイト来訪者の統計を取るために Google Analytics を使っています。これが個人を識別可能な情報(IP アドレス等)を集めていることを GDPR に沿う形で明示するとともに、サイト来訪者がしかるべきアクション(クッキー拒否など)を取れるような仕組みが厳密には求められます。
こういった要件とどう付き合っていくかは今後の課題になるだろうということで、ここに記しておきます

@nard-tech
Copy link
Contributor Author

@mikkame

(3) (2) でダウンロードした Excel ファイルをパースして JSON に展開する機能
の件,Excel ファイルの表の構造は現状 tool/convert.php から推測できるので既にバレバレという気もします…

ちょっと今から一旦流れを整理しますね.

@nard-tech
Copy link
Contributor Author

nard-tech commented Apr 5, 2020

@mcdmaster コメントありがとうございます.PHP -> TS の件についてはその通りだと思います.

GDPR の件も把握しました.ガバナンス,コンプライアンスの観点からは確かに重要なことで,ある意味非機能要件ではあると思うのですが,#1471 のようにこれからも活発に議論がなされると思われるので,この issue で本格的に議論されるよりは,専用の issue を立てて頂いた方がよいと思います.
この issue だと他の話題と混ざって議論しづらくなってしまいますし.

@nard-tech
Copy link
Contributor Author

@mcdmaster
この issue を立てた意図としては,現状の問題点を洗い出すとともに,非機能要件の改善に着手するときに重要となる優先度を見極め,適切な手順を決めたい,というところがあります.

プライバシーの件はそれ単体で議論して必要な実装を進めて頂いても問題ないと思います.

もちろん,連携は取らせて頂きたいので,issue を立てたら「cf. #2790」などと書いておいて頂ければと思います.

@mcdmaster
Copy link
Contributor

@nard-tech りょうかいしました。イシューをフォークします、という機能があるといいなと思いました(笑)

それはさておき、非機能要件のおおよその三大要素はパフォーマンス、セキュリティ、運用というふうに市中企業では言われたりします。もちろん、運用のところがインフラや DR だったりのように他の形でのグルーピングもあると思います。
とはいえ、おっしゃるように、セキュリティやそれに密接に関連するガバナンス、コンプライアンス、あるいはユーザ教育といったあたりにまで話を広げると、プロジェクトとしてはいきおい手に余らせてしまいますよね。
なので、いわゆる GRC プラス IT セキュリティ的な話は、都のポリシや標準、手順、ガイドラインも既にあるはずですので、疎かにならない程度に活動を続けていくこととします。その中から、これがオープンソース的じゃん!みたいなモノが出せるのが究極の nice-to-have ですね

@nard-tech
Copy link
Contributor Author

とりあえず皆さんが思っている問題点は可視化できたと思いますので,とりあえず

については私が着手したいと思います.

終わり次第,

にも着手します.

についても,進捗がないようであれば私がやってしまおうと思います.

については,削除してもよろしいでしょうか (@mikkame)

@nard-tech
Copy link
Contributor Author

nard-tech commented Apr 6, 2020

data.json の分割については,変換スクリプトを書き換えて GitHub で公開する件とも関わってきます.
変換スクリプトを書き換えるには,@mikkame さんにお任せするか,#2790 (comment) で記した通り,ファイルごとの責務を分割して可能な限り公開して頂いて誰か手の空いている方にお願いすることになると思います.
いずれにせよ,@mikkame さんの手を煩わせることになってしまいますが,今後の作業を @mikkame さんに属人化させず誰でも修正・変更できるようにした方が好ましいと思いますので,宜しくお願い致します.

@mcdmaster
Copy link
Contributor

これ、私がやってみたいですね。大人の事情(?)的なガバナンス、コンプライアンス、法令順守なんていうのはここ数年の専門分野だったりするのですけど、元 Java エンジニアとしてはかなり興味を持っています。
とはいえ、もはやロートル(笑)なのでスピード感にはあまり期待しないでください。それじゃ厳しいよ…ということであれば、もちろん進んで他の方にお譲りします

@kaizumaki
Copy link
Collaborator

変換スクリプトについてですが、元Excelが原則非公開であるとともに、運営側でも今後データの運用が変わる可能性があります。ですので、変換スクリプトの書き換えについてはペンディングにしてもらえますよう、お願いいたします。

@nard-tech
Copy link
Contributor Author

@hikyaru-suzuki ありがとうございます.型情報が入ったので,いろいろとやりやすくなる気がします.

PR にコメントしていますので,ご確認下さい.

@mcdmaster
Copy link
Contributor

mcdmaster commented Apr 8, 2020

#2955 を持ってきたのですけれども、ここには2個の非機能要件があると思っています。

  • ルック&フィールのカイゼン
  • サービスデスク的な仕組み

前者は、このサイトの元々の目的の(と、私が勝手に思っている)データを正確かつタイムリーに、都民および関係者に伝えるという使命からすると、極端に言えば無視できる類のものでもあります。しかしながら、対応の仕方いかんではサイトの、あるいはコントリビューターの皆さんの評価を落とすことにつながりかねない要件でもあります。

この種の cosmetic problem など軽微なイシューの受付は、この GitHub のイシューに最終的には載せるとしても、いわゆる一次受け的な仕組みが必要になってきたのではないかと考えています。ITIL 的な L1, L2, L3 アプローチですね。

これらについては、 #2955 に discussion タグをつけた上で、別スレとして議論していこうと思います

@nard-tech
Copy link
Contributor Author

nard-tech commented Apr 10, 2020

@mcdmaster
先にも述べた通り,

この issue を立てた意図としては,現状の問題点を洗い出すとともに,非機能要件の改善に着手するときに重要となる優先度を見極め,適切な手順を決めたい,というところがあります.

(#2790 (comment))

「機能要件以外は非機能要件」と言われてしまうと返す言葉がありませんが,非機能要件だからといって何でも話題に挙げられてしまうと収拾がつかなくなりますので,その点,ご配慮お願いします.

@nard-tech nard-tech changed the title 非機能要件 問題点・改善点の整理 非機能要件(可読性・拡張性・設計) 問題点・改善点の整理 Apr 10, 2020
@mcdmaster
Copy link
Contributor

本イシューの目的につき、理解しました。また、タイトルの修正も確認しました。
まさに、私が各所でプロマネをやっていた頃、ユーザさんの「非機能要件」に関する要望への対応に苦労させられたのと同じ思いをさせてしまいましたね。失礼しました

nard-tech added a commit to nard-tech/covid19-tokyo that referenced this issue May 7, 2020
nard-tech added a commit to nard-tech/covid19-tokyo that referenced this issue May 7, 2020
nard-tech added a commit to nard-tech/covid19-tokyo that referenced this issue Aug 14, 2020
…ass-data-to-template

Conflicts:
  components/SiteTopUpper.vue
  components/cards/ConfirmedCasesByMunicipalitiesCard.vue
  components/cards/SevereCaseCard.vue
  components/cards/TestedNumberCard.vue
soutaito added a commit that referenced this issue Aug 17, 2020
…a-to-template

template に data.json をそのまま渡している箇所があるため,<script> タグ内で template に渡す値を限定する
soutaito added a commit that referenced this issue May 26, 2021
…y-of-components

component のディレクトリ構成を変更する
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
discussion 議論が目的、または議論中の Issue improvement 改善や新機能の要望
Projects
None yet
Development

No branches or pull requests

6 participants