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

[Server] CosmosTemporaryExposureKeyRepositoryの日付でのフィルタリングが本来の意図と異なる可能性がある #973

Closed
keiji opened this issue Apr 14, 2022 · 0 comments · Fixed by #1021
Labels
bug バグ。本来あるべき動作をしていないもの confirmed 開発内部管理用

Comments

@keiji
Copy link
Collaborator

keiji commented Apr 14, 2022

不具合の内容 / Describe the bug

サーバー側のクラスCosmosTemporaryExposureKeyRepositoryのメソッドにGetNextAsyncGetOutOfTimeKeysAsyncがある。
GetNextAsyncは、診断キーの生成対象となるTEKだけを取得する役割を担い、GetOutOfTimeKeysAsyncは期限切れのTEKを取得する役割がある。

public async Task<TemporaryExposureKeyModel[]> GetNextAsync()
{
_logger.LogInformation($"start {nameof(GetNextAsync)}");
var oldest = (int)new DateTimeOffset(DateTime.UtcNow.AddDays(Constants.OutOfDateDays).Date.Ticks, TimeSpan.Zero).ToUnixTimeSeconds() / 600;
var query = _db.TemporaryExposureKey.GetItemLinqQueryable<TemporaryExposureKeyModel>(true)
.Where(tek => tek.RollingStartIntervalNumber > oldest)
.Where(tek => tek.Exported == false)
.ToFeedIterator();
var e = Enumerable.Empty<TemporaryExposureKeyModel>();
while (query.HasMoreResults)
{
var r = await query.ReadNextAsync();
e = e.Concat(r.Resource);
}
return e.ToArray();
}
public async Task<TemporaryExposureKeyModel[]> GetOutOfTimeKeysAsync()
{
_logger.LogInformation($"start {nameof(GetOutOfTimeKeysAsync)}");
var oldest = (int)new DateTimeOffset(DateTime.UtcNow.AddDays(Constants.OutOfDateDays).Date.Ticks, TimeSpan.Zero).ToUnixTimeSeconds() / 600;
var query = _db.TemporaryExposureKey.GetItemLinqQueryable<TemporaryExposureKeyModel>(true)
.Where(tek => tek.RollingStartIntervalNumber < oldest)
.ToFeedIterator();
var e = Enumerable.Empty<TemporaryExposureKeyModel>();
while (query.HasMoreResults)
{
var r = await query.ReadNextAsync();
e = e.Concat(r.Resource);
}
return e.ToArray();
}

どちらの処理も大きな違いはなく、Cosmos DBからTemporaryExposureKeyを取得後、基準日(oldest)より新しいか、過去のものかでフィルタリングする点に違いがある(他にも条件に若干差があるが、ここでは割愛する)。

問題となるのは、フィルタリングの条件式(42行目, 58行目)。
どちらも > < となっていて等号を含まないため、基準日と完全に一致するRollingStartIntervalNumberがあれば、どちらのケースにも該当しないとして取り扱われると考える。

期待される挙動 / Expected behavior

GetNextAsyncの方の条件式は >= とする。

その他 / Additional context

履歴を見る限り、COVID19Radar時代からこうなっていた様子。

配信している診断キーを外部から観察しているが、14日前の診断キーも変換されているのがわかる。100%発生するというものではなさそう。
(COCOAが送信するTEKは診断日または症状が現れた日より2日(時差を考慮して3日)前からなので、14日前の診断キーが送られるケースの全体に占める割合は小さい。

keiji/cocoa-statistics@22d6cdf

確認および修正のためにCosmosTemporaryExposureKeyRepositoryのユニットテストを書こうと試みたが、CosmosDBやLINQに阻まれてうまくテストが書けていない。


Internal IDs:
-Bug 6797

@keiji keiji added bug バグ。本来あるべき動作をしていないもの waiting-for-confirmation 関係者に確認中のもの labels Apr 14, 2022
@cocoa-dev008 cocoa-dev008 added confirmed 開発内部管理用 and removed waiting-for-confirmation 関係者に確認中のもの labels Apr 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug バグ。本来あるべき動作をしていないもの confirmed 開発内部管理用
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants