Skip to content
This repository has been archived by the owner on Jun 24, 2022. It is now read-only.

Add download entire period bundle feature #176

Merged
merged 3 commits into from
Jul 14, 2020
Merged

Add download entire period bundle feature #176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d08cd81
Add 00000 option
maxneuvians Jul 13, 2020
7f9e366
Add feature flag
maxneuvians Jul 13, 2020
ac74376
Small fixes and tests
maxneuvians Jul 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,5 @@ hmacKeyLength: 32
corsAccessControlAllowOrigin: "*"

# Feature flags
disableCurrentDateCheckFeatureFlag: true
disableCurrentDateCheckFeatureFlag: true
enableEntirePeriodBundle: true
32 changes: 17 additions & 15 deletions pkg/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,22 @@ import (
var log = logger.New("config")

type Constants struct {
DefaultSubmissionServerPort uint32
DefaultRetrievalServerPort uint32
DefaultServerPort uint32
WorkerExpirationInterval uint32
MaxOneTimeCode int64
MaxConsecutiveClaimKeyFailures int
ClaimKeyBanDuration uint32
MaxDiagnosisKeyRetentionDays uint32
InitialRemainingKeys uint32
EncryptionKeyValidityDays uint32
OneTimeCodeExpiryInMinutes uint32
AssignmentParts int
HmacKeyLength int
CORSAccessControlAllowOrigin string
DisableCurrentDateCheckFeatureFlag bool
DefaultSubmissionServerPort uint32
DefaultRetrievalServerPort uint32
DefaultServerPort uint32
WorkerExpirationInterval uint32
MaxOneTimeCode int64
MaxConsecutiveClaimKeyFailures int
ClaimKeyBanDuration uint32
MaxDiagnosisKeyRetentionDays uint32
InitialRemainingKeys uint32
EncryptionKeyValidityDays uint32
OneTimeCodeExpiryInMinutes uint32
AssignmentParts int
HmacKeyLength int
CORSAccessControlAllowOrigin string
DisableCurrentDateCheckFeatureFlag bool
EnableEntirePeriodBundle bool
}

var AppConstants Constants
Expand Down Expand Up @@ -62,4 +63,5 @@ func setDefaults() {
viper.SetDefault("hmacKeyLength", 32)
viper.SetDefault("corsAccessControlAllowOrigin", "*")
viper.SetDefault("disableCurrentDateCheckFeatureFlag", true)
viper.SetDefault("enableEntirePeriodBundle", false)
}
38 changes: 28 additions & 10 deletions pkg/server/retrieve.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,24 +68,42 @@ func (s *retrieveServlet) retrieve(w http.ResponseWriter, r *http.Request) resul
return s.fail(log(ctx, nil).WithField("method", r.Method), w, "method not allowed", "", http.StatusMethodNotAllowed)
}

dateNumber64, err := strconv.ParseUint(vars["day"], 10, 32)
if err != nil {
return s.fail(log(ctx, err), w, "invalid day parameter", "", http.StatusBadRequest)
}
dateNumber := uint32(dateNumber64)
var startTimestamp time.Time
var endTimestamp time.Time
var dateNumber uint32

if config.AppConstants.EnableEntirePeriodBundle == true && vars["day"] == "00000" {

endDate := timemath.CurrentDateNumber() - 1
startDate := endDate - numberOfDaysToServe

dateNumber = endDate

startTimestamp = time.Unix(int64(startDate*86400), 0)
endTimestamp = time.Unix(int64((endDate+1)*86400), 0)
Copy link

Choose a reason for hiding this comment

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

Maybe I'm reading this wrong, but it seems like we're doing "yesterday" on L77, then taking numberOfDaysToServe in the past, and then on L83 we're adding back the day we removed so we're back to "today"
This seems to imply that we're also serving today as part of the bundle, is there ever a use case where we'd want DisableCurrentDateCheckFeatureFlag to be false and EnableEntirePeriodBundle to be true? It seems like we're assuming that EnableEntirePeriodBundle means DisableCurrentDateCheckFeatureFlag is also true.
It's also entirely possible that my timeMath is just bad.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

endTimestamp is just the outer bound of the period, so you go up to the end days midnight. We need to ensure that yesterdays day is used in startDate and dateNumber and that the correct endTimestamp is then calculated.


startTimestamp := time.Unix(int64(dateNumber*86400), 0)
endTimestamp := time.Unix(int64((dateNumber+1)*86400), 0)
} else {

dateNumber64, err := strconv.ParseUint(vars["day"], 10, 32)
if err != nil {
return s.fail(log(ctx, err), w, "invalid day parameter", "", http.StatusBadRequest)
}
dateNumber = uint32(dateNumber64)

startTimestamp = time.Unix(int64(dateNumber*86400), 0)
endTimestamp = time.Unix(int64((dateNumber+1)*86400), 0)

}

currentRSIN := pb.CurrentRollingStartIntervalNumber()
currentDateNumber := timemath.CurrentDateNumber()

if config.AppConstants.DisableCurrentDateCheckFeatureFlag == false && dateNumber == currentDateNumber {
return s.fail(log(ctx, err), w, "request for current date", "cannot serve data for current period for privacy reasons", http.StatusNotFound)
return s.fail(log(ctx, nil), w, "request for current date", "cannot serve data for current period for privacy reasons", http.StatusNotFound)
Copy link

Choose a reason for hiding this comment

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

Got it, we're checking it here, so we do need both to be true for it to work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

} else if dateNumber > currentDateNumber {
return s.fail(log(ctx, err), w, "request for future data", "cannot request future data", http.StatusNotFound)
return s.fail(log(ctx, nil), w, "request for future data", "cannot request future data", http.StatusNotFound)
} else if dateNumber < (currentDateNumber - numberOfDaysToServe) {
return s.fail(log(ctx, err), w, "request for too-old data", "requested data no longer valid", http.StatusGone)
return s.fail(log(ctx, nil), w, "request for too-old data", "requested data no longer valid", http.StatusGone)
}

// TODO: Maybe implement multi-pack linked-list scheme depending on what we hear back from G/A
Expand Down
44 changes: 44 additions & 0 deletions test/retrieve_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,50 @@ def test_retrieve_stuff
assert_keys(export, keys, region: 'CA', date_number: dn)
end

def test_all_keys
active_at = time_in_date('10:00', today_utc.prev_day(8))
two_days_ago = yesterday_utc.prev_day(1)
fourteen_days_ago = yesterday_utc.prev_day(13)
fiveteen_days_ago = yesterday_utc.prev_day(14)

# Our retrieve endpoint returns keys SUBMITTED within the given period.
Copy link

Choose a reason for hiding this comment

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

Should we check the bounds? Like 14 days ago and today as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added!

add_key(active_at: active_at, submitted_at: time_in_date("23:59:59", fiveteen_days_ago), data: '1' * 16)
add_key(active_at: active_at, submitted_at: time_in_date("00:00", fourteen_days_ago), data: '2' * 16)
add_key(active_at: active_at, submitted_at: time_in_date("01:59:59", yesterday_utc), data: '3' * 16)
add_key(active_at: active_at, submitted_at: time_in_date("02:00", yesterday_utc), data: '4' * 16)
add_key(active_at: active_at, submitted_at: time_in_date("02:00", yesterday_utc), data: '5' * 16)
add_key(active_at: active_at, submitted_at: time_in_date("02:00", today_utc), data: '6' * 16)

rsin = rolling_start_interval_number(active_at)

resp = get_date("00000")

config = get_app_config()

if config["enableEntirePeriodBundle"]
export = assert_happy_zip_response(resp)
keys = [tek(
rolling_start_interval_number: rsin,
transmission_risk_level: 8,
data: "2222222222222222",
), tek(
rolling_start_interval_number: rsin,
transmission_risk_level: 8,
data: "3333333333333333",
), tek(
rolling_start_interval_number: rsin,
transmission_risk_level: 8,
data: "4444444444444444",
), tek(
rolling_start_interval_number: rsin,
transmission_risk_level: 8,
data: "5555555555555555",
)]
else
assert_response(resp, 410, 'text/plain; charset=utf-8', body: "requested data no longer valid\n")
end
end

def test_period_bounds
active_at = time_in_date('10:00', today_utc.prev_day(8))
two_days_ago = yesterday_utc.prev_day(1)
Expand Down