fix(daemon): small refactor of notices api (#337)

Most importantly, wait to lock the state until after the optional duration has
been parsed, in case an error occurs. Also, moves the types query
parsing/sanitization into its own function, since this cleans up the GET
notices handler and better matches future handling of user ID filtering.

Signed-off-by: Oliver Calder <[email protected]>

internals/daemon/api_notices.go

@@ -42,18 +42,8 @@ type addedNotice struct {
 func v1GetNotices(c *Command, r *http.Request, _ *UserState) Response {
 	query := r.URL.Query()
 
-	typeStrs := strutil.MultiCommaSeparatedList(query["types"])
-	types := make([]state.NoticeType, 0, len(typeStrs))
-	for _, typeStr := range typeStrs {
-		noticeType := state.NoticeType(typeStr)
-		if !noticeType.Valid() {
-			// Ignore invalid notice types (so requests from newer clients
-			// with unknown types succeed).
-			continue
-		}
-		types = append(types, noticeType)
-	}
-	if len(types) == 0 && len(typeStrs) > 0 {
+	types, err := sanitizeTypesFilter(query["types"])
+	if err != nil {
 		// Caller did provide a types filter, but they're all invalid notice types.
 		// Return no notices, rather than the default of all notices.
 		return SyncResponse([]*state.Notice{})
@@ -71,16 +61,18 @@ func v1GetNotices(c *Command, r *http.Request, _ *UserState) Response {
 		Keys:  keys,
 		After: after,
 	}
-	var notices []*state.Notice
-
-	st := c.d.overlord.State()
-	st.Lock()
-	defer st.Unlock()
 
 	timeout, err := parseOptionalDuration(query.Get("timeout"))
 	if err != nil {
 		return statusBadRequest("invalid timeout: %v", err)
 	}
+
+	st := c.d.overlord.State()
+	st.Lock()
+	defer st.Unlock()
+
+	var notices []*state.Notice
+
 	if timeout != 0 {
 		// Wait up to timeout for notices matching given filter to occur
 		ctx, cancel := context.WithTimeout(r.Context(), timeout)
@@ -106,6 +98,24 @@ func v1GetNotices(c *Command, r *http.Request, _ *UserState) Response {
 	return SyncResponse(notices)
 }
 
+func sanitizeTypesFilter(queryTypes []string) ([]state.NoticeType, error) {
+	typeStrs := strutil.MultiCommaSeparatedList(queryTypes)
+	types := make([]state.NoticeType, 0, len(typeStrs))
+	for _, typeStr := range typeStrs {
+		noticeType := state.NoticeType(typeStr)
+		if !noticeType.Valid() {
+			// Ignore invalid notice types (so requests from newer clients
+			// with unknown types succeed).
+			continue
+		}
+		types = append(types, noticeType)
+	}
+	if len(types) == 0 && len(typeStrs) > 0 {
+		return nil, errors.New("all requested notice types invalid")
+	}
+	return types, nil
+}
+
 func v1PostNotices(c *Command, r *http.Request, _ *UserState) Response {
 	var payload struct {
 		Action      string          `json:"action"`