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

Feat: プラグインのスクリプトとデータを端末間で同期をできるように #12302

Open
wants to merge 19 commits into
base: develop
Choose a base branch
from

Conversation

slofp
Copy link
Contributor

@slofp slofp commented Nov 10, 2023

What

プラグインのスクリプトと設定、Mk:loadMk:saveなどの情報を端末間で同期をできるようにしました。また、Mk:loadMk:saveを端末間で共有するにあたりWidgetとPlayもMk:loadMk:saveのデータを端末間で共有できるようにしました。

Why

resolve #12209, resolve #11919

Additional info (optional)

Checklist

  • Read the contribution guide
  • Test working in a local environment
  • (If needed) Add story of storybook
  • (If needed) Update CHANGELOG.md
  • (If possible) Add tests

@github-actions github-actions bot added packages/frontend Client side specific issue/PR packages/backend Server side specific issue/PR labels Nov 10, 2023
Copy link

codecov bot commented Nov 10, 2023

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (4b3f9bd) 78.80% compared to head (65afaac) 78.81%.

Files Patch % Lines
packages/backend/src/core/RegistryApiService.ts 0.00% 4 Missing ⚠️
...i/endpoints/i/registry/remove-all-keys-in-scope.ts 94.87% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #12302   +/-   ##
========================================
  Coverage    78.80%   78.81%           
========================================
  Files          946      947    +1     
  Lines       102271   102318   +47     
  Branches      8263     8265    +2     
========================================
+ Hits         80596    80637   +41     
- Misses       21675    21681    +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

github-actions bot commented Nov 10, 2023

このPRによるapi.jsonの差分

差分はこちら
--- base
+++ head
@@ -36312,6 +36312,159 @@
         }
       }
     },
+    "/i/registry/remove-all-keys-in-scope": {
+      "post": {
+        "operationId": "i/registry/remove-all-keys-in-scope",
+        "summary": "i/registry/remove-all-keys-in-scope",
+        "description": "No description provided.\n\n**Credential required**: *Yes*",
+        "externalDocs": {
+          "description": "Source code",
+          "url": "https://github.com/misskey-dev/misskey/blob/develop/packages/backend/src/server/api/endpoints/i/registry/remove-all-keys-in-scope.ts"
+        },
+        "security": [
+          {
+            "ApiKeyAuth": []
+          }
+        ],
+        "requestBody": {
+          "required": true,
+          "content": {
+            "application/json": {
+              "schema": {
+                "type": "object",
+                "properties": {
+                  "scope": {
+                    "type": "array",
+                    "default": [],
+                    "items": {
+                      "type": "string",
+                      "pattern": "^[a-zA-Z0-9_]+$"
+                    }
+                  },
+                  "domain": {
+                    "type": "string",
+                    "nullable": true
+                  }
+                },
+                "required": [
+                  "scope"
+                ]
+              }
+            }
+          }
+        },
+        "responses": {
+          "204": {
+            "description": "OK (without any results)"
+          },
+          "400": {
+            "description": "Client error",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/Error"
+                },
+                "examples": {
+                  "INVALID_PARAM": {
+                    "value": {
+                      "error": {
+                        "message": "Invalid param.",
+                        "code": "INVALID_PARAM",
+                        "id": "3d81ceae-475f-4600-b2a8-2bc116157532"
+                      }
+                    }
+                  }
+                }
+              }
+            }
+          },
+          "401": {
+            "description": "Authentication error",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/Error"
+                },
+                "examples": {
+                  "CREDENTIAL_REQUIRED": {
+                    "value": {
+                      "error": {
+                        "message": "Credential required.",
+                        "code": "CREDENTIAL_REQUIRED",
+                        "id": "1384574d-a912-4b81-8601-c7b1c4085df1"
+                      }
+                    }
+                  }
+                }
+              }
+            }
+          },
+          "403": {
+            "description": "Forbidden error",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/Error"
+                },
+                "examples": {
+                  "AUTHENTICATION_FAILED": {
+                    "value": {
+                      "error": {
+                        "message": "Authentication failed. Please ensure your token is correct.",
+                        "code": "AUTHENTICATION_FAILED",
+                        "id": "b0a7f5f8-dc2f-4171-b91f-de88ad238e14"
+                      }
+                    }
+                  }
+                }
+              }
+            }
+          },
+          "418": {
+            "description": "I'm Ai",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/Error"
+                },
+                "examples": {
+                  "I_AM_AI": {
+                    "value": {
+                      "error": {
+                        "message": "You sent a request to Ai-chan, Misskey's showgirl, instead of the server.",
+                        "code": "I_AM_AI",
+                        "id": "60c46cd1-f23a-46b1-bebe-5d2b73951a84"
+                      }
+                    }
+                  }
+                }
+              }
+            }
+          },
+          "500": {
+            "description": "Internal server error",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/Error"
+                },
+                "examples": {
+                  "INTERNAL_ERROR": {
+                    "value": {
+                      "error": {
+                        "message": "Internal error occurred. Please contact us if the error persists.",
+                        "code": "INTERNAL_ERROR",
+                        "id": "5d37dbcb-891e-41ca-a3d6-e690c97775ac"
+                      }
+                    }
+                  }
+                }
+              }
+            }
+          }
+        }
+      }
+    },
     "/i/registry/set": {
       "post": {
         "operationId": "i/registry/set",

Get diff files from Workflow Page

@slofp
Copy link
Contributor Author

slofp commented Nov 11, 2023

概要を細かく分解した説明

長いので折りたたみ
  • プラグインは今までのローカルと、アカウントの2つの保存形態を持ちます。
    • ローカル: 今までのように端末間の同期はなくすべてがローカルに保存されます。(後述するMk:savetoAccount: trueにした場合もローカルに保存されます。)
    • アカウント: 端末間でプラグイン、プラグイン設定、データを同期します。アカウントベースではプラグインはインストール時にハッシュが生成され、重複を起こす場合はユーザーにアクションを選択させます。(例えばプラグインアップデートの場合はデータを残したままコードのみを変更できる)
      image
      • アカウントプラグインはpluginsに格納されます
        image
  • また、今までに保存されているプラグインはアカウントには移行せずにローカルプラグインとして保存されます。(つまり何もしない)
    • プラグインの管理にはそのプラグインがアカウントプラグインの場合は名前の横にバッチが付きます(現時点では「同期」としています。)
      image
  • ローカルプラグインはアカウントプラグインに移行ができます。
    image
    • 移行する際に引き継がれるものはプラグイン本体とプラグイン設定のみです。(データはそれがローカル用なのかアカウント用なのかがわからないため)
    • 移行しようとしているプラグインがすでにアカウントに存在する場合は上書きをするかを確認します。確定をするとアカウントに存在したそのプラグイン本体、プラグイン設定、プラグインのデータは削除され、プラグイン本体、プラグイン設定が移行元の状態を引き継ぎます。
  • データの保存に使用されるMk:loadMk:saveには 新たにtoAccount?: boolという引数を追加します。 オプションオブジェクトを追加します。
    • ※ 具体的にはMk:load(key toAccount?), Mk:save(key value toAccount?)となる
    • ※ 具体的にはMk:load(key option?), Mk:save(key value option?)となる
      • optiontoAccount?: boolがあり、trueにするとアカウントにデータが保存される
    • これはプラグインに限らずPlay、Widgetにも適用されます。
    • レジストリの保存方法は現状のlocalStorageのキーのaiscript:plugins:<id>:<key>と同じようになるようにスコープを指定しています。
      image

ハッシュについて

  • ハッシュはプラグインの名前と作成者を使用しています(この2要素はそんなに変更されてないはず?です)。

@slofp slofp marked this pull request as ready for review November 12, 2023 13:29
@syuilo
Copy link
Member

syuilo commented Nov 13, 2023

具体的にはMk:load(key toAccount?), Mk:save(key value toAccount?)となる

最後の引数はオプションオブジェクトにした方が拡張性が高そう

@syuilo
Copy link
Member

syuilo commented Nov 13, 2023

プラグインってIDが設定されていたと思うけど、それは使わずにハッシュを新たに生成するのはなぜかしら?

@slofp
Copy link
Contributor Author

slofp commented Nov 13, 2023

プラグインってIDが設定されていたと思うけど、それは使わずにハッシュを新たに生成するのはなぜかしら?

<2023.11.0時点のコード>

プラグインをインストールするときに生成されるidはuuidで一意性は保証されていますが

  • 重複チェックができない
    (現状含む)ローカルではプラグインを重複して入れられます。ローカルともかく端末間となると重複して入れられるのはそのプラグインの誤作動の発生や不要なデータが溜まり続ける等があるので。また、プラグイン側で何かしら修正などのアップデートがあったときにハッシュによって既存のデータを参照してコードだけを変えることに使用しています。
  • uuidは長い
    uuidは128bit列なのでユーザー内だけで完結するものでは長すぎると思います。もう一つはuuid自体にハイフンを含むためスコープには使うのは不適切だと思いました。(現状のスコープ指定の規則も^[a-zA-Z0-9_]+$なので)

@syuilo
Copy link
Member

syuilo commented Nov 15, 2023

あー今ってプラグインのID設定必須じゃないのか

import { $i } from '@/account.js';

type ScriptType = 'widget' | 'plugins' | 'flash';
export type ScriptData = { type: ScriptType; id?: string; fromAccount?: boolean };
Copy link
Member

Choose a reason for hiding this comment

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

ScriptDataという名前だとどういうものが入るのか分かりにくいかも

Copy link
Contributor Author

Choose a reason for hiding this comment

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

scriptMetadataだと本来のメタデータと被ってしまうのでstorageMetadataとかですかね... storageの保存・読み込みする際に使う情報という意味で

@tai-cha

This comment was marked as outdated.

@tai-cha
Copy link
Contributor

tai-cha commented Nov 15, 2023

畳まれている部分をめちゃくちゃ見逃しているコメントをしました、良さそう

@tai-cha
Copy link
Contributor

tai-cha commented Nov 15, 2023

プラグインのidが指定されている場合にはそちらを優先するような挙動はさすがに複雑すぎるかしら

@tai-cha
Copy link
Contributor

tai-cha commented Nov 15, 2023

uuidは128bit列なのでユーザー内だけで完結するものでは長すぎる

MisskeyHubにプラグインストアができる可能性があるのでユーザー内で完結しない可能性がある
misskey-dev/misskey-hub#2
misskey-dev/misskey-hub-next#21

ただ、MisskeyHubではそれはそれで別にこのidと同じものを使わなくてもいいので別に問題はないか

@slofp
Copy link
Contributor Author

slofp commented Nov 15, 2023

プラグインのidが指定されている場合にはそちらを優先するような挙動はさすがに複雑すぎるかしら

これを考えた初めVSCodeのextension IDみたいなの(author.pluginnameぽいもの)をメタデータに入れて使えればと思ったんですが、idを管理するところがないのでユーザーが指定した際にどこかでidを被らせてしまう可能性と、以前のプラグインはidが指定されていない?(そもそも昔のmisskey-hubにはidの説明がなかったのでほとんどの人は気にしてなかったはず?)ので、プラグインストアができるのであればメタデータにidがあればたしかに優先したほうが良さそうですね

@slofp
Copy link
Contributor Author

slofp commented Nov 15, 2023

ただあまりハイフンが入ってるidを入れるのであればscopeのフォーマット制限を緩和させるか、そもそもidにハイフンが入らないようにするかの2択になると思います。(idに区切りとして.を入れる分にはそこをscopeの区切りにすればいいため)

@slofp
Copy link
Contributor Author

slofp commented Nov 17, 2023

メタデータにidが指定されていればそちらを優先するようにしました🙏

@tai-cha
Copy link
Contributor

tai-cha commented Apr 16, 2024

やっぱり欲しい気がする

@slofp
Copy link
Contributor Author

slofp commented Apr 16, 2024

コンフリ解決も含めて何かもう少し改善できそうな点がありそうなら改善した上でしたいですね

もうほぼ半年前のものですし

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/backend Server side specific issue/PR packages/frontend Client side specific issue/PR
Projects
3 participants