notifier: documentation and simplification pass

Signed-off-by: Hank Donnay <hdonnay@redhat.com>

httptransport/notificationshandler.go

@@ -92,9 +92,9 @@ func (h *NotifHandler) Get(w http.ResponseWriter, r *http.Request) {
 	}
 
 	// optional page_size parameter
-	var pageSize uint64
+	var pageSize int
 	if param := r.URL.Query().Get("page_size"); param != "" {
-		pageSize, err = strconv.ParseUint(param, 10, 64)
+		p, err := strconv.ParseInt(param, 10, 64)
 		if err != nil {
 			resp := &je.Response{
 				Code:    "bad-request",
@@ -103,6 +103,7 @@ func (h *NotifHandler) Get(w http.ResponseWriter, r *http.Request) {
 			je.Error(w, resp, http.StatusBadRequest)
 			return
 		}
+		pageSize = int(p)
 	}
 	if pageSize == 0 {
 		pageSize = DefaultPageSize

notifier/delivery.go

@@ -71,8 +71,7 @@ func (d *Delivery) deliver(ctx context.Context) error {
 		case <-ticker.C:
 			zlog.Debug(ctx).
 				Msg("delivery tick")
-			err := d.RunDelivery(ctx)
-			if err != nil {
+			if err := d.RunDelivery(ctx); err != nil {
 				zlog.Error(ctx).
 					Err(err).
 					Msg("encountered error on tick")

notifier/notification.go

@@ -5,7 +5,7 @@ import (
 	"github.com/quay/claircore"
 )
 
-// Reason indicates the catalyst for a notification
+// Reason indicates the catalyst for a notification.
 type Reason string
 
 const (
@@ -14,17 +14,14 @@ const (
 	Changed Reason = "changed"
 )
 
-// Notification summarizes a change in the
-// vulnerabilities affecting a manifest
+// Notification summarizes a change in the vulnerabilities affecting a manifest.
 //
-// The mentioned Vulnerability will be the
-// most severe vulnerability discovered in an
-// update operation.
+// The mentioned Vulnerability will be the most severe vulnerability discovered
+// in an update operation.
 //
-// Receiving clients are expected to filter
-// notifications by severity in such a way
-// that they receive all vulnerabilities at or
-// above a particular claircore.Severity level.
+// Receiving clients are expected to filter notifications by severity in such a
+// way that they receive all vulnerabilities at or above a particular
+// claircore.Severity level.
 type Notification struct {
 	ID            uuid.UUID        `json:"id"`
 	Manifest      claircore.Digest `json:"manifest"`

notifier/pager.go

@@ -4,10 +4,10 @@ import (
 	"github.com/google/uuid"
 )
 
-// Page communicates a bare-minimum paging procotol with clients
+// Page communicates a bare-minimum paging protocol with clients.
 type Page struct {
-	// the max number of elements returned in a page
-	Size uint64 `json:"size"`
 	// the next id to retrieve
 	Next *uuid.UUID `json:"next,omitempty"`
+	// the max number of elements returned in a page
+	Size int `json:"size"`
 }

notifier/poller.go

@@ -20,16 +20,15 @@ const (
 // PollerOpt applies a configuration to a Poller
 type PollerOpt func(*Poller) error
 
-// Poller implements new Update Operation discovery via
-// an event channel.
+// Poller implements new Update Operation discovery via an event channel.
 type Poller struct {
-	// the interval to poll a Matcher node.
-	interval time.Duration
 	// a store to retrieve known UOIDs and compare
 	// with the polled UOIDs.
 	store Store
 	// a differ to retrieve latest update operations
 	differ matcher.Differ
+	// the interval to poll a Matcher node.
+	interval time.Duration
 }
 
 func NewPoller(interval time.Duration, store Store, differ matcher.Differ) *Poller {
@@ -40,8 +39,8 @@ func NewPoller(interval time.Duration, store Store, differ matcher.Differ) *Poll
 	}
 }
 
-// Event is delivered on the poller's channel when
-// a new UpdateOperation is discovered.
+// Event is delivered on the poller's channel when a new UpdateOperation is
+// discovered.
 type Event struct {
 	updater string
 	uo      driver.UpdateOperation
@@ -89,8 +88,8 @@ func (p *Poller) poll(ctx context.Context, c chan<- Event) {
 	}
 }
 
-// onTick retrieves the latest update operations for all known
-// updaters and delivers an event if notification creation is necessary.
+// OnTick retrieves the latest update operations for all known updaters and
+// delivers an event if notification creation is necessary.
 func (p *Poller) onTick(ctx context.Context, c chan<- Event) {
 	ctx = zlog.ContextWithValues(ctx, "component", "notifier/Poller.onTick")
 

notifier/postgres/notification_pagination_test.go

@@ -23,7 +23,7 @@ func TestNotePagination(t *testing.T) {
 		// total number of notifications to request
 		total int
 		// number of notifications per page to test
-		pageSize uint64
+		pageSize int
 	}{
 		{
 			name:     "check total zero",
@@ -40,7 +40,6 @@ func TestNotePagination(t *testing.T) {
 			total:    1,
 			pageSize: 1,
 		},
-
 		{
 			name:     "check odds 1",
 			total:    3,
@@ -97,7 +96,7 @@ func TestNotePagination(t *testing.T) {
 				t.Fatalf("failed to insert notifications: %v", err)
 			}
 
-			var inPage = notifier.Page{
+			inPage := notifier.Page{
 				Size: tt.pageSize,
 			}
 
@@ -112,7 +111,7 @@ func TestNotePagination(t *testing.T) {
 				if outPage.Size != tt.pageSize {
 					t.Fatalf("got: %v, want: %v", outPage.Size, tt.pageSize)
 				}
-				if uint64(len(returned)) > tt.pageSize {
+				if len(returned) > tt.pageSize {
 					t.Fatalf("got: %v, want: %v", len(returned), tt.pageSize)
 				}
 				returned, outPage, err = store.Notifications(ctx, noteID, &outPage)

notifier/postgres/notifications.go

@@ -103,7 +103,7 @@ func notifications(ctx context.Context, pool *pgxpool.Pool, id uuid.UUID, page *
 		return nil, notifier.Page{}, clairerror.ErrBadNotification{id, err}
 	}
 
-	morePages := uint64(len(notifications)) == limit
+	morePages := len(notifications) == limit
 	if morePages {
 		// Slice off the last element as it was only an indicator
 		// that another page should be delivered.

notifier/processor.go

@@ -24,12 +24,6 @@ import (
 // Processor(s) create atomic boundaries, no two Processor(s) will be creating
 // notifications for the same UOID at once.
 type Processor struct {
-	// NoSummary controls whether per-manifest vulnerability summarization
-	// should happen.
-	NoSummary bool
-	// NoSummary is a little awkward to use, but reversing the boolean this way
-	// makes the defaults line up better.
-
 	// distributed lock used for mutual exclusion
 	locks Locker
 	// a handle to an indexer service
@@ -40,6 +34,12 @@ type Processor struct {
 	store Store
 	// a integer id used for logging
 	id int
+
+	// NoSummary controls whether per-manifest vulnerability summarization
+	// should happen.
+	//
+	// The zero value makes the default behavior to do the summary.
+	NoSummary bool
 }
 
 func NewProcessor(id int, l Locker, indexer indexer.Service, matcher matcher.Service, store Store) *Processor {
@@ -52,9 +52,8 @@ func NewProcessor(id int, l Locker, indexer indexer.Service, matcher matcher.Ser
 	}
 }
 
-// Process is an async method which receives new UOs as events,
-// creates notifications, persists these notifications,
-// and updates the notifier system with the "latest" seen UOID.
+// Process receives new UOs as events, creates and persists notifications, and
+// updates the notifier system with the "latest" seen UOID.
 //
 // Canceling the ctx will end the processing.
 func (p *Processor) Process(ctx context.Context, c <-chan Event) {
@@ -196,8 +195,8 @@ func min(a, b int) int {
 // It has supporting structures for concurrent use and summaries.
 type notifTab struct {
 	sync.Mutex
-	N      []Notification
 	lookup map[string]int // only used in "summary" mode
+	N      []Notification
 }
 
 // GetAffected issues AffectedManifest calls in chunks and merges the result.

notifier/store.go

@@ -62,7 +62,7 @@ type Notificationer interface {
 	// with a notification id.
 	//
 	// Normally Receipter.SetDeleted will be issues first, however
-	// application logic may decide to gc notifications which have not been
+	// application logic may decide to GC notifications which have not been
 	// set deleted after some period of time, thus this condition should not
 	// be checked.
 	DeleteNotifications(ctx context.Context, id uuid.UUID) error