Add feature migrations mechanism #165

keiji · 2021-05-13T05:08:26Z

Issue 番号 / Issue ID

Close バージョンアップ時に移行（マイグレーション）する仕組みを作る #156

目的 / Purpose

アップデート時のメンテナンスコストを低減するため、バージョンアップ（アップデート）時に、特定のバージョンから最新バージョンまでの移行処理を一元的に行う仕組みを用意する。

破壊的変更をもたらしますか / Does this introduce a breaking change?

[x] Yes
[ ] No

Pull Request の種類 / Pull Request type

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

検証方法 / How to test

コードの入手 / Get the code

git clone [repo-address]
cd [repo-name]
git checkout [branch-name]
dotnet restore

コードの検証 / Test the code

確認事項 / What to check

バージョン1.2.2未満 -> 1.2.2のアップデート
バージョン1.2.2 -> 1.2.3のアップデート

その他 / Other information

検証用の実証。

マイグレーション

マイグレーション関係の処理は各プロジェクトのServices/Migration内に集約している。

Covid19Radar/Services/Migration/MigrationService.cs マイグレーションを統括するMigrationServiceが定義されている。
IMigrationProcessServiceには、マイグレーションが必要となるバージョンについてメソッドとして規定する（マイグレーションが発生しないバージョンは加えなくて良い）。
IMigrationProcessServiceは、IoCを通じてプラットフォーム固有のマイグレーション処理をMigrationServiceに提供する。

マイグレーションの実行は、MigrationServiceのメソッドMigrateAsync(Version? fromVersion)で行う。
現在のバージョン（fromVersion）に対してどのバージョンのマイグレーションが必要かを判断して実行する。

マイグレーションは、2つの要素からなる。
一つは共通プロジェクト側のデータのマイグレーション（たとえばApplicationPropertyServiceからPreferenceへの移行）。
もう一つはプラットフォーム固有のマイグレーション（たとえばWorkManagerのキャンセル処理）。

プラットフォーム固有のマイグレーションは、各プラットフォームプロジェクトが、IoCを通じてIMigrationProcessServiceの実装として提供する。

マイグレーションの実行タイミング

Android

マイグレーションは2つのタイミングで実行される。

一つ目は、アプリがアップデートしたタイミング。これはアプリがアップデートしたタイミングでシステムが呼び出すIntent（ android.intent.action.MY_PACKAGE_REPLACED ）を受け取るAppVersionUpgradeReceiverを通じて実行される。
二つ目は、バックグラウンドでの診断キーの取得と接触確認を行うバックグルアンド処理内。

iOS

マイグレーションは2つのタイミングで実行される。

一つ目は、アプリが起動したタイミング。
二つ目は、バックグラウンドでの診断キーの取得と接触確認を行うバックグラウンド処理内。

Covid19Radar/Covid19Radar/Services/VersionMigrationService.cs

Covid19Radar/Tests/Covid19Radar.UnitTests/Services/Migration/VersionMigrationServiceTests.cs

keiji · 2021-05-17T10:13:55Z

二日寝かせたらもっといい方法らしきものを思いついたのでやってみる。

keiji · 2021-05-17T16:01:02Z

もっといい方法というのは、バージョンごとのMigratorをリストに積んで順次実行するものだったのだけど、実際にやろうとしたらうまくいかなかった。
共通処理だけであれば問題ないけど、Platform固有のMigratorを呼ぶとなると処理が複雑になりすぎるので、このままの方が良いという結論に。

keiji · 2021-06-11T02:12:40Z

しばらく寝かしたらもっといい方法が思い浮かぶかと思ったけどそうでもなかったのでReady for reviewした。

keiji · 2021-07-11T03:07:16Z

rebased

Covid19Radar/Covid19Radar.Android/MainApplication.cs

Covid19Radar/Covid19Radar.Android/Services/Migration/Migrator123.cs

Covid19Radar/Covid19Radar/Services/Migration/Migrator122.cs

Covid19Radar/Covid19Radar/Services/Migration/VersionMigrationService.cs

…gration

keiji · 2021-07-30T01:08:08Z

ひとまずconflictを解消

cocoa-dev · 2021-07-30T05:13:39Z

@keiji
簡単でいいのでマイグレーション処理の内容を PR の概要に記載していただくことは可能でしょうか？

…gration

WorkManagerMigrator will be executed as Setup on migration process. Because latest worker registration must be only once.

…adcastReceiver will be killed by the system.

cocoa-dev · 2021-08-26T03:18:51Z

@keiji
PR の概要にマイグレーション処理について記載してくださり、ありがとうございます。

マイグレーション処理の実行タイミングについて 2 点ほど理由を確認させてください。
回答は PR の概要を編集していただく形で構いません。

iOS でアプリの起動時にマイグレーション処理を実行するのは、Android と異なりアプリのバージョンアップをフックできないからでしょうか？
バックグラウンド処理内でマイグレーション処理を実行するのはなぜでしょうか？

Covid19Radar/Covid19Radar.Android/Services/Migration/MigrationProcessService.cs

Covid19Radar/Covid19Radar.iOS/Services/Migration/MigrationProcessService.cs

Covid19Radar/Covid19Radar/Services/Migration/MigrationService.cs

Covid19Radar/Covid19Radar/ViewModels/HomePage/SplashPageViewModel.cs

cocoa-dev · 2021-08-27T05:51:04Z

Covid19Radar/Covid19Radar/Services/Migration/MigrationService.cs

+            return VERSION_1_0_0;
+        }
+
+        private async Task MigrateAsync(Version? fromVersion)


以下のような警告がMac版のVisualStudioで出ていますが期待通りになっているでしょうか？

理由があってnullableにしているので接触的に明示していきたいところですが、#nullable を置かないと警告が表示される。これをどうするかという話ですね。

今のところコード規約がないので、警告の解消を優先すれば最小限の範囲で#nullableを付けるのも良いかなと思いました。
しかしその場合、あとから規約としてクラス全体に#nullableを付ける（プロジェクト設定で一括指定したり？）など齟齬が発生した場合に差分が大きくなりますので、コード規約が定まってから手当てしても遅くはないだろうなと思って現状にした経緯があります。

開発チーム内で現状、 #nullable に関する指針が無ければ、警告は一旦このままにしておいて、将来的に方針が定まり次第解消という方向が良いかなと思いますが、いかがでしょうか。

限定的な指定も含めてどちらでも対応できるので、ご意見をお聞かせください。

開発チーム内で現状、 #nullable に関する指針が無ければ

はい。現在指針はありません。
このコードとして理想的な実装になっていたほうがいいとも思いますが、たしかに指針があるわけではないので一旦このままで良いです。

ずいぶん前にもらってる #35 にもありますね。
いっそプロジェクトファイルでNull許容を有効にしてしまっても良いような気がしますが、このPRでやるのが良いかと言われれば 🤔 です。

とはいえ色のつく警告を放置は気持ち悪いので、クラスファイルの最初と最後に#nullable enable/disableで対応しました。

cocoa-dev · 2021-08-27T05:53:11Z

Covid19Radar/Covid19Radar/Services/Migration/MigrationService.cs

+                    _loggerService.Debug($"appVersion entry is not found in Preferences.");
+                    return null;
+                }
+                var appVersion = _preferencesService.GetValue<string>(PreferenceKey.AppVersion, FIRST_VERSION);


こちらもVisuslStudioにより修正候補が出ていますが、<string>部分は省略可です。

cocoa-dev · 2021-08-27T06:05:43Z

Covid19Radar/Covid19Radar/Services/Migration/MigrationService.cs

+            var fromVersion = PreferenceVersion;
+            fromVersion = fromVersion is null ? GuessVersion() : fromVersion;


var fromVersion = PreferenceVersion ?? GuessVersion();としたほうがいいと思います。

cocoa-dev · 2021-08-27T06:16:30Z

Covid19Radar/Covid19Radar/Services/Migration/MigrationService.cs

+                }
+            }
+
+            var currentVersion = CurrentAppVersion;


ローカル変数に入れる必要は無いと思います

あ、すみません。CurrentAppVersionはプロパティですが都度読み込み処理がされるんですね。

各プロパティはコンストラクタのタイミングのみで読み込むのでは都合悪いでしょうか？
できるならそのほうがわかりやすいかと思います。
都度読み込む必要があるならメソッドにしたほうがわかりやすいかもしれません。

UpdateAppVersionPreferenceで値を書き換える場合があるので、常に最新の値を読み込めるようにしています。
その上で、当該処理ではローカル変数currentVersionに格納してしまっていますし、UpdateAppVersionPreferenceとの対象もわかりづらいので良くないですね。

この部分は考え直します。

CurrentAppVersionはアプリのバージョンなので固定でした。
コンストラクタからEssentialServiceを使うとエラーが起きてスタックすることがわかったので、呼び出しのときに変数を取って使い回すようにしてあります。

UpdateAppVersionPreferenceもわかりにくかったのでSet/Getの組み合わせのメソッドを用意しました。

cocoa-dev · 2021-08-27T06:27:33Z

Covid19Radar/Covid19Radar/Services/Migration/MigrationService.cs

+            }
+        }
+
+        private Task<bool> DetectDowngradeAsync()


非同期で動作するようなものではないので、戻り値boolのメソッドで良いと思います。

ダウングレードなしで行くことになったので、このメソッドは削除になりました。

cocoa-dev · 2021-08-27T06:36:32Z

Covid19Radar/Covid19Radar/Services/Migration/MigrationService.cs

+            if (await DetectDowngradeAsync())
+            {
+                _loggerService.Warning("Downgrade is detected. All data will be cleared.");
+
+                await ClearAllDataAsync();
+
+                _loggerService.Info("All data cleared.");
+            }


ダウングレード時のみということは、一般ユーザ向けとしては不要なコードですね。
デバッグ用としても、ダウングレードする際は一旦アンインストールするべきで不要なコードだと思います。

ありがとうございます。
Androidの場合は常に正式リリースのバージョンは上がるばかりで、下がることはない（ダウングレードはない）という認識です。

一方、ぼくはiOSの事はよくわかっていないのですが、そちらも同様でしょうか。AppStoreからバージョン指定して落とせるみたいな事はなさそうでしょうか。

ないようであれば当該部分は削除します。

はい。iOSでも同じくダウングレードはできないと思います。

ダウングレードはないのでこの周辺の処理は削除します。
念のためログだけ仕込んでおきます。

keiji · 2021-08-27T07:05:32Z

iOS でアプリの起動時にマイグレーション処理を実行するのは、Android と異なりアプリのバージョンアップをフックできないからでしょうか？

はい。その認識ですが、確認はしておいた方がいいですね。
@kazuhiro4949 iOSでは、アプリのアップグレードのタイミングをフックして処理する方法は提供されていますか？

バックグラウンド処理内でマイグレーション処理を実行するのはなぜでしょうか？

これはExposureWindow modeの際に指摘をもらったのですが「BroadcastReceiverであまり長い時間処理をすると、システムによって強制停止される可能性がある」というAndroid的な事情に対応するためです。

時間としてはだいたい10秒で、今回のマイグレーション処理でこれを越えることはなさそうなのですが、しかし今後どうなるかわからないので安全策としてWorkManagerを使っています。

一方、ご指摘の通り、アプリ起動時に発生するマイグレーションとアップデート時のBroadcastReceiverから起動するマイグレーションが衝突する可能性はありますね。
ありがとうございます。排他機構を入れるなどして、マイグレーションが終わるまではスプラッシュ画面で待ってもらうことを検討します（そうするとスプラッシュ画面に「データ移行中です」のようなステータス表示が欲しくなりますが…）。

…ProcessService.cs Co-authored-by: cocoa-dev <69558847+cocoa-dev@users.noreply.github.com>

…referenceVersion.

IEssentialsService could not use in constructor.

…on_migration

…ice.

cocoa-dev · 2021-08-30T00:47:01Z

Covid19Radar/Covid19Radar.Android/Services/Migration/WorkManagerMigrator.cs

+            string[] oldWorkName = {
+                "exposurenotification",
+            };


こちら内容が変わってしまっていますのでご対応をお願いします。

そうか……この間のsyncでここ変わったんですね。対応します！

Covid19Radar/Covid19Radar/Services/Migration/Migrator_1_2_2.cs

keiji force-pushed the version_migration branch from a8e3819 to 15c0571 Compare May 13, 2021 05:30

keiji commented May 13, 2021

View reviewed changes

Covid19Radar/Covid19Radar/Services/VersionMigrationService.cs Outdated Show resolved Hide resolved

keiji self-assigned this May 13, 2021

keiji force-pushed the version_migration branch from cf0c96e to b938439 Compare May 15, 2021 03:34

keiji commented May 15, 2021

View reviewed changes

Covid19Radar/Tests/Covid19Radar.UnitTests/Services/Migration/VersionMigrationServiceTests.cs Outdated Show resolved Hide resolved

keiji changed the title ~~Implement migration~~ Add feature migrations mechanism May 15, 2021

keiji changed the base branch from master to develop June 7, 2021 07:20

keiji marked this pull request as ready for review June 11, 2021 02:11

keiji requested a review from cocoa-dev June 11, 2021 02:12

keiji force-pushed the version_migration branch from 43eb2c1 to e48849a Compare July 11, 2021 03:05

keiji force-pushed the version_migration branch from e48849a to 2e79bdb Compare July 11, 2021 03:13

keiji added 2 commits July 26, 2021 17:03

Add feature migrations mechanism.

181a06e

Add a feature recovery data that corrupted datetime format

e5a925f

keiji force-pushed the version_migration branch from 2e79bdb to e5a925f Compare July 26, 2021 08:36

cocoa-dev suggested changes Jul 29, 2021

View reviewed changes

Merge branch 'develop' of github.com:cocoa-mhlw/cocoa into version_mi…

a05c888

…gration

keiji added 8 commits August 9, 2021 00:05

Merge branch 'develop' of github.com:cocoa-mhlw/cocoa into version_mi…

ed6b414

…gration

Fix merge

cf4d41f

Changed class name to more suitable

e5b4dca

Fix build

230f5c0

Add WorkManagerMigrator

6fada52

WorkManagerMigrator will be executed as Setup on migration process. Because latest worker registration must be only once.

Refactoring

a342325

On Android, Migration process run on WorkManager for prevent that Bro…

7987643

…adcastReceiver will be killed by the system.

Class names are simplified

cbdfe25

cocoa-dev suggested changes Aug 26, 2021

View reviewed changes

cocoa-dev reviewed Aug 27, 2021

View reviewed changes

keiji and others added 11 commits August 29, 2021 18:44

Update Covid19Radar/Covid19Radar.Android/Services/Migration/Migration…

8d75510

…ProcessService.cs Co-authored-by: cocoa-dev <69558847+cocoa-dev@users.noreply.github.com>

Replace PreferenceVersion and UpdateAppVersionPreference to Set/Get P…

b086214

…referenceVersion.

Remove downgrade process.

8364f0c

Resolve warning.

4e7566a

Refacotirng.

ae98777

Fix SplashScreen stacked.

6b19ed5

IEssentialsService could not use in constructor.

Add log

d80b770

Merge branch 'version_migration' of github.com:keiji/cocoa into versi…

e953b8d

…on_migration

Change abstract class AbsMigrationService to interface IMigrationServ…

fb2711b

…ice.

Commented.

56d8e82

Merge cocoa-dev suggested change.

83b8324

keiji requested a review from cocoa-dev August 29, 2021 13:01

cocoa-dev reviewed Aug 30, 2021

View reviewed changes

Fix missing old worker name.

076ab51

keiji requested a review from cocoa-dev August 30, 2021 02:08

cocoa-dev reviewed Aug 30, 2021

View reviewed changes

Covid19Radar/Covid19Radar/Services/Migration/Migrator_1_2_2.cs Outdated Show resolved Hide resolved

Change locale-dependent datetime to UTC.

51e1613

keiji requested a review from cocoa-dev August 30, 2021 04:11

cocoa-dev approved these changes Aug 30, 2021

View reviewed changes

keiji merged commit 55277b0 into cocoa-mhlw:develop Aug 30, 2021

keiji deleted the version_migration branch August 30, 2021 08:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add feature migrations mechanism #165

Add feature migrations mechanism #165

keiji commented May 13, 2021 •

edited

Loading

keiji commented May 17, 2021

keiji commented May 17, 2021

keiji commented Jun 11, 2021

keiji commented Jul 11, 2021

keiji commented Jul 30, 2021

cocoa-dev commented Jul 30, 2021

cocoa-dev commented Aug 26, 2021

cocoa-dev Aug 27, 2021

keiji Aug 27, 2021

cocoa-dev Aug 27, 2021 •

edited

Loading

keiji Aug 27, 2021

keiji Aug 29, 2021

cocoa-dev Aug 27, 2021

keiji Aug 30, 2021

cocoa-dev Aug 27, 2021

keiji Aug 30, 2021

cocoa-dev Aug 27, 2021

cocoa-dev Aug 27, 2021

cocoa-dev Aug 27, 2021

keiji Aug 27, 2021

keiji Aug 29, 2021

cocoa-dev Aug 27, 2021

keiji Aug 30, 2021

cocoa-dev Aug 27, 2021

keiji Aug 27, 2021

cocoa-dev Aug 27, 2021

keiji Aug 27, 2021

keiji commented Aug 27, 2021 •

edited

Loading

cocoa-dev Aug 30, 2021

keiji Aug 30, 2021

		var fromVersion = PreferenceVersion;
		fromVersion = fromVersion is null ? GuessVersion() : fromVersion;

Add feature migrations mechanism #165

Add feature migrations mechanism #165

Conversation

keiji commented May 13, 2021 • edited Loading

Issue 番号 / Issue ID

目的 / Purpose

破壊的変更をもたらしますか / Does this introduce a breaking change?

Pull Request の種類 / Pull Request type

検証方法 / How to test

コードの入手 / Get the code

コードの検証 / Test the code

確認事項 / What to check

その他 / Other information

マイグレーション

マイグレーションの実行タイミング

Android

iOS

keiji commented May 17, 2021

keiji commented May 17, 2021

keiji commented Jun 11, 2021

keiji commented Jul 11, 2021

keiji commented Jul 30, 2021

cocoa-dev commented Jul 30, 2021

cocoa-dev commented Aug 26, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cocoa-dev Aug 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keiji commented Aug 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keiji commented May 13, 2021 •

edited

Loading

cocoa-dev Aug 27, 2021 •

edited

Loading

keiji commented Aug 27, 2021 •

edited

Loading