Merge pull request #283 from smallstep/max/empty-oids-nil

Always convert empty list to nil when saving orderIDs index.

acme/account_test.go

@@ -524,6 +524,43 @@ func Test_getOrderIDsByAccount(t *testing.T) {
 				res: []string{"o2", "o4"},
 			}
 		},
+		"ok/no-pending-orders": func(t *testing.T) test {
+			oids := []string{"o1"}
+			boids, err := json.Marshal(oids)
+			assert.FatalError(t, err)
+
+			invalidOrder, err := newO()
+			assert.FatalError(t, err)
+			invalidOrder.Status = StatusInvalid
+			binvalidOrder, err := json.Marshal(invalidOrder)
+			assert.FatalError(t, err)
+
+			return test{
+				id: "foo",
+				db: &db.MockNoSQLDB{
+					MGet: func(bucket, key []byte) ([]byte, error) {
+						switch string(bucket) {
+						case string(ordersByAccountIDTable):
+							assert.Equals(t, key, []byte("foo"))
+							return boids, nil
+						case string(orderTable):
+							return binvalidOrder, nil
+						default:
+							assert.FatalError(t, errors.Errorf("did not expect query to table %s", bucket))
+							return nil, nil
+						}
+					},
+					MCmpAndSwap: func(bucket, key, old, newval []byte) ([]byte, bool, error) {
+						assert.Equals(t, bucket, ordersByAccountIDTable)
+						assert.Equals(t, key, []byte("foo"))
+						assert.Equals(t, old, boids)
+						assert.Nil(t, newval)
+						return nil, true, nil
+					},
+				},
+				res: []string{},
+			}
+		},
 	}
 	for name, run := range tests {
 		t.Run(name, func(t *testing.T) {

acme/order.go

@@ -125,10 +125,17 @@ func newOrder(db nosql.DB, ops OrderOptions) (*order, error) {
 
 type orderIDs []string
 
+// save is used to update the list of orderIDs keyed by ACME account ID
+// stored in the database.
+//
+// This method always converts empty lists to 'nil' when storing to the DB. We
+// do this to avoid any confusion between an empty list and a nil value in the
+// db.
 func (oids orderIDs) save(db nosql.DB, old orderIDs, accID string) error {
 	var (
 		err  error
 		oldb []byte
+		newb []byte
 	)
 	if len(old) == 0 {
 		oldb = nil
@@ -138,9 +145,13 @@ func (oids orderIDs) save(db nosql.DB, old orderIDs, accID string) error {
 			return ServerInternalErr(errors.Wrap(err, "error marshaling old order IDs slice"))
 		}
 	}
-	newb, err := json.Marshal(oids)
-	if err != nil {
-		return ServerInternalErr(errors.Wrap(err, "error marshaling new order IDs slice"))
+	if len(oids) == 0 {
+		newb = nil
+	} else {
+		newb, err = json.Marshal(oids)
+		if err != nil {
+			return ServerInternalErr(errors.Wrap(err, "error marshaling new order IDs slice"))
+		}
 	}
 	_, swapped, err := db.CmpAndSwap(ordersByAccountIDTable, []byte(accID), oldb, newb)
 	switch {

acme/order_test.go

@@ -516,7 +516,7 @@ func Test_newOrder(t *testing.T) {
 	}
 }
 
-func TestOrderIDsSave(t *testing.T) {
+func TestOrderIDs_save(t *testing.T) {
 	accID := "acc-id"
 	newOids := func() orderIDs {
 		return []string{"1", "2"}
@@ -591,6 +591,26 @@ func TestOrderIDsSave(t *testing.T) {
 				},
 			}
 		},
+		"ok/new-empty-saved-as-nil": func(t *testing.T) test {
+			oldOids := newOids()
+			oids := []string{}
+
+			oldb, err := json.Marshal(oldOids)
+			assert.FatalError(t, err)
+			return test{
+				oids: oids,
+				old:  oldOids,
+				db: &db.MockNoSQLDB{
+					MCmpAndSwap: func(bucket, key, old, newval []byte) ([]byte, bool, error) {
+						assert.Equals(t, old, oldb)
+						assert.Equals(t, newval, nil)
+						assert.Equals(t, bucket, ordersByAccountIDTable)
+						assert.Equals(t, key, []byte(accID))
+						return nil, true, nil
+					},
+				},
+			}
+		},
 	}
 	for name, run := range tests {
 		t.Run(name, func(t *testing.T) {