Repository: google/exposure-notifications-server
Pull request #1188 (2020-11-23 17:15:18)

Remove dash, sort by ASCII to int
**Release Note**

<!-- Enter your extended release note in the below block. If the PR requires
additional action from users switching to the new release, include the string
"action required". If no release note is required, write "NONE". -->

```release-note
Remove dash in export filenames, sort by ASCII to int
```

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/1188#" title="Author self-approved">sethvargo</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)~~ [sethvargo]

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":[]} -->
sethvargo: /assign @mikehelmick 
jeremyfaller: /lgtm

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/1188#" title="Author self-approved">sethvargo</a>*, *<a href="https://github.com/google/exposure-notifications-server/pull/1188#pullrequestreview-536706723" 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)~~ [sethvargo,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":[]} -->
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/1188#pullrequestreview-536707509" title="Approved">mikehelmick</a>*, *<a href="https://github.com/google/exposure-notifications-server/pull/1188#" title="Author self-approved">sethvargo</a>*, *<a href="https://github.com/google/exposure-notifications-server/pull/1188#pullrequestreview-536706723" 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,sethvargo,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":[]} -->
google-oss-robot: New changes are detected. LGTM label has been removed.
google-oss-robot: @sethvargo: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-en-server-release-unit | ce24164714570c3f08bc229bf4f9ae92c0fc586c | [link](https://oss-prow.knative.dev/view/gs/oss-prow/pr-logs/pull/google_exposure-notifications-server/1188/pull-en-server-release-unit/1330931463863930880) | `/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: /retest
sethvargo: /rest
sethvargo: /retest

Patch:
diff --git a/internal/export/worker.go b/internal/export/worker.go
index ae3bb9e..945a75b 100644
--- a/internal/export/worker.go
+++ b/internal/export/worker.go
@@ -457,7 +457,23 @@ func exportFilename(eb *model.ExportBatch, batchNum int, pbHexSHA string) string
 	if len(pbHexSHA) >= 6 {
 		first6 = pbHexSHA[0:6]
 	}
-	return fmt.Sprintf("%s/%d-%d-%05d-%s%s", eb.FilenameRoot, eb.StartTimestamp.Unix(), eb.EndTimestamp.Unix(), batchNum, first6, filenameSuffix)
+
+	// Convert the sha to it's 3-digit ASCII equivalent. This is required because
+	// some app developers hard-coded a regular expression which assumes only
+	// digits in filenames.
+	first6 = toASCIISortable(first6)
+
+	return fmt.Sprintf("%s/%d-%d-%05d999999999%s%s", eb.FilenameRoot, eb.StartTimestamp.Unix(), eb.EndTimestamp.Unix(), batchNum, first6, filenameSuffix)
+}
+
+// toASCIISortable converts each character in the provided string to its
+// ASCII-digit-only equivalent string (of numbers), padded to 3 digits.
+func toASCIISortable(s string) string {
+	var result string
+	for _, r := range s {
+		result = fmt.Sprintf("%s%03d", result, int(r))
+	}
+	return result
 }
 
 func exportIndexFilename(eb *model.ExportBatch) string {


Test patch:
diff --git a/internal/export/worker_test.go b/internal/export/worker_test.go
index f6692a8..3820f15 100644
--- a/internal/export/worker_test.go
+++ b/internal/export/worker_test.go
@@ -20,6 +20,7 @@ import (
 	"time"
 
 	"github.com/google/exposure-notifications-server/internal/database"
+	"github.com/google/exposure-notifications-server/internal/export/model"
 	publishdb "github.com/google/exposure-notifications-server/internal/publish/database"
 	publishmodel "github.com/google/exposure-notifications-server/internal/publish/model"
 	"github.com/google/exposure-notifications-server/internal/serverenv"
@@ -438,3 +439,82 @@ func TestVariableBatchMaxSize(t *testing.T) {
 		}
 	}
 }
+
+func TestExportFilename(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		m   *model.ExportBatch
+		num int
+		sha string
+		exp string
+	}{
+		{
+			m: &model.ExportBatch{
+				FilenameRoot:   "v1",
+				StartTimestamp: time.Unix(0, 0),
+				EndTimestamp:   time.Unix(0, 0),
+			},
+			num: 1,
+			sha: "abc123",
+			exp: "v1/0-0-00001999999999097098099049050051.zip",
+		},
+		{
+			m: &model.ExportBatch{
+				FilenameRoot:   "v1",
+				StartTimestamp: time.Unix(0, 0),
+				EndTimestamp:   time.Unix(0, 0),
+			},
+			num: 2,
+			sha: "",
+			exp: "v1/0-0-00002999999999.zip",
+		},
+		{
+			m: &model.ExportBatch{
+				FilenameRoot:   "v2",
+				StartTimestamp: time.Unix(100, 0),
+				EndTimestamp:   time.Unix(300, 0),
+			},
+			num: 1,
+			sha: "",
+			exp: "v2/100-300-00001999999999.zip",
+		},
+	}
+
+	for _, tc := range cases {
+		tc := tc
+
+		t.Run(tc.sha, func(t *testing.T) {
+			t.Parallel()
+
+			if got, want := exportFilename(tc.m, tc.num, tc.sha), tc.exp; got != want {
+				t.Errorf("expected %q to be %q", got, want)
+			}
+		})
+	}
+}
+
+func TestToASCIISortable(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		in  string
+		out string
+	}{
+		{"foo", "102111111"},
+		{"bar", "098097114"},
+		{"ad3e93", "097100051101057051"},
+	}
+
+	for _, tc := range cases {
+		tc := tc
+
+		t.Run(tc.in, func(t *testing.T) {
+			t.Parallel()
+
+			if got, want := toASCIISortable(tc.in), tc.out; got != want {
+				t.Errorf("expected %q to be %q", got, want)
+			}
+		})
+	}
+}