Repository: google/exposure-notifications-server
Pull request #1161 (2020-11-12 00:41:17)

allow a revised key to come in again, as long is it no changey
## Proposed Changes

* If a revised key is uploaded again and the reportType hasn't changed, don't error.

**Release Note**

```release-note
 If a revised key is uploaded again and the reportType hasn't changed, don't error.
```

google-oss-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/google/exposure-notifications-server/pull/1161#" title="Author self-approved">mikehelmick</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=google%2Fexposure-notifications-server).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/google/exposure-notifications-server/blob/main/OWNERS)~~ [mikehelmick]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
google-oss-robot: @mikehelmick: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-en-server-release-unit | 3d22544d0c6ea91def280bf350ee24258eb02c04 | [link](https://oss-prow.knative.dev/view/gs/oss-prow/pr-logs/pull/google_exposure-notifications-server/1161/pull-en-server-release-unit/1326686477362401280) | `/test pull-en-server-release-unit`

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
google-oss-robot: @mikehelmick: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-en-server-release-unit | f860badf815ef48379fa49e5327da9bb0a99b12e | [link](https://oss-prow.knative.dev/view/gs/oss-prow/pr-logs/pull/google_exposure-notifications-server/1161/pull-en-server-release-unit/1326689143152971776) | `/test pull-en-server-release-unit`

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
sethvargo: Is this to work around a client issue? It feels like a weird behavior to enable...
mikehelmick: > Is this to work around a client issue? It feels like a weird behavior to enable...

Currently - if a client attempts to upload unchanged keys multiple times that works.
And if you revise, and then try to keep uploading the revised state, it doesn't work.

This is necessary to enable additional key uploads after you have revised to a confirmed diagnosis from likely.

Not a CUJ any clients are covering, but it is one the server supports.
google-oss-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/google/exposure-notifications-server/pull/1161#" title="Author self-approved">mikehelmick</a>*, *<a href="https://github.com/google/exposure-notifications-server/pull/1161#pullrequestreview-529432253" title="Approved">whaught</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=google%2Fexposure-notifications-server).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/google/exposure-notifications-server/blob/main/OWNERS)~~ [mikehelmick,whaught]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/internal/publish/model/exposure_model.go b/internal/publish/model/exposure_model.go
index 2f5d682..ff92b94 100644
--- a/internal/publish/model/exposure_model.go
+++ b/internal/publish/model/exposure_model.go
@@ -173,6 +173,10 @@ func (e *Exposure) Revise(in *Exposure) (bool, error) {
 	if e.ReportType == in.ReportType {
 		return false, nil
 	}
+	// Key is being published again, but has already been revised to target report type.
+	if e.RevisedAt != nil && e.RevisedReportType != nil && *e.RevisedReportType == in.ReportType {
+		return false, nil
+	}
 	if !e.LocalProvenance {
 		nonLocalOK := false
 		if e.ExportImportID != nil {


Test patch:
diff --git a/internal/publish/database/exposure_test.go b/internal/publish/database/exposure_test.go
index 5cdac02..c01b31d 100644
--- a/internal/publish/database/exposure_test.go
+++ b/internal/publish/database/exposure_test.go
@@ -577,6 +577,9 @@ func TestReviseExposures(t *testing.T) {
 			t.Errorf("expected %d to be %d: %#v", got, want, reviseResp)
 		}
 
+		// Change report, type, attempt to revise again.
+		exposure.ReportType = verifyapi.ReportTypeNegative
+
 		// Attempt to revise the same key again - this should fail.
 		if _, err := pubDB.InsertAndReviseExposures(ctx, &InsertAndReviseExposuresRequest{
 			Incoming:     []*model.Exposure{exposure},
diff --git a/internal/publish/model/exposure_model_test.go b/internal/publish/model/exposure_model_test.go
index 3e6e920..46a6252 100644
--- a/internal/publish/model/exposure_model_test.go
+++ b/internal/publish/model/exposure_model_test.go
@@ -1536,6 +1536,42 @@ func TestExposureReview(t *testing.T) {
 			needsRevision: true,
 			err:           "",
 		},
+		{
+			name: "double_revise",
+			previous: &Exposure{
+				ReportType:            verifyapi.ReportTypeClinical,
+				LocalProvenance:       true,
+				HealthAuthorityID:     int64Ptr(2),
+				Regions:               []string{"US", "CA"},
+				TransmissionRisk:      4,
+				CreatedAt:             createdAt,
+				DaysSinceSymptomOnset: int32Ptr(-1),
+				RevisedAt:             &createdAt,
+				RevisedReportType:     stringPtr(verifyapi.ReportTypeConfirmed),
+			},
+			incoming: &Exposure{
+				ReportType:            verifyapi.ReportTypeConfirmed,
+				HealthAuthorityID:     int64Ptr(3),
+				Regions:               []string{"MX"},
+				TransmissionRisk:      5,
+				CreatedAt:             revisedAt,
+				DaysSinceSymptomOnset: int32Ptr(0),
+			},
+			want: &Exposure{
+				ReportType:                   verifyapi.ReportTypeClinical,
+				LocalProvenance:              true,
+				HealthAuthorityID:            int64Ptr(3),
+				Regions:                      []string{"US", "CA", "MX"},
+				TransmissionRisk:             4,
+				CreatedAt:                    createdAt,
+				DaysSinceSymptomOnset:        int32Ptr(-1),
+				RevisedReportType:            stringPtr(verifyapi.ReportTypeConfirmed),
+				RevisedAt:                    &revisedAt,
+				RevisedDaysSinceSymptomOnset: int32Ptr(0),
+				RevisedTransmissionRisk:      intPtr(5),
+			},
+			needsRevision: false,
+		},
 	}
 
 	for _, tc := range cases {
diff --git a/internal/publish/publish_test.go b/internal/publish/publish_test.go
index 8b6a6f5..0f80773 100644
--- a/internal/publish/publish_test.go
+++ b/internal/publish/publish_test.go
@@ -862,9 +862,9 @@ func TestKeyRevision(t *testing.T) {
 					t.Fatal(err)
 				}
 
-				// Revise them
+				// Revise them to negative - test will try to revise to positive.
 				for _, key := range incoming {
-					key.ReportType = verifyapi.ReportTypeConfirmed
+					key.ReportType = verifyapi.ReportTypeNegative
 				}
 				if _, err := pubDB.InsertAndReviseExposures(ctx, &pubdb.InsertAndReviseExposuresRequest{
 					Incoming: incoming,