Skip to content

Commit

Permalink
api/v1/firmware_sets: firmware set updates should persist attribute i…
Browse files Browse the repository at this point in the history 
…nformation

This change splits up the serverComponentFirmwareSetUpdate() method
and implements some checks to ensure the firmware set attribute data
does not get overwritten to be empty as part of an update, unless its expected.

Adds additional tests to cover for firmware set attributes updates.
  • Loading branch information
@joelrebel
joelrebel committed Aug 16, 2023
1 parent 6a5a33c commit ac2c6eb
Show file tree
Hide file tree
Showing 2 changed files with 157 additions and 32 deletions.
63 changes: 40 additions & 23 deletions pkg/api/v1/router_firmware_set.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ func (r *Router) firmwareSetMap(ctx context.Context, firmwareSet *models.Compone
return m, nil
}

func (r *Router) firmwareSetUpdateTx(ctx context.Context, newValues *models.ComponentFirmwareSet, attributes models.AttributesFirmwareSetSlice, firmwareUUIDs []uuid.UUID) error {
func (r *Router) firmwareSetUpdateTx(ctx context.Context, fwSetUpdate *models.ComponentFirmwareSet, attrsUpdate models.AttributesFirmwareSetSlice, firmwareUUIDs []uuid.UUID) error {
// being transaction to update a firmware set and its references
tx, err := r.DB.BeginTx(ctx, nil)
if err != nil {
Expand All @@ -446,41 +446,30 @@ func (r *Router) firmwareSetUpdateTx(ctx context.Context, newValues *models.Comp
// nolint:errcheck // TODO(joel): log error
defer tx.Rollback()

currentValues, err := models.FindComponentFirmwareSet(ctx, tx, newValues.ID)
fwSetCurrent, err := models.FindComponentFirmwareSet(ctx, tx, fwSetUpdate.ID)
if err != nil {
fmt.Println(err)
return err
}

// update name column
if newValues.Name != "" && newValues.Name != currentValues.Name {
currentValues.Name = newValues.Name
if fwSetUpdate.Name != "" && fwSetUpdate.Name != fwSetCurrent.Name {
fwSetCurrent.Name = fwSetUpdate.Name
}

if _, err := currentValues.Update(ctx, tx, boil.Infer()); err != nil {
if _, err := fwSetCurrent.Update(ctx, tx, boil.Infer()); err != nil {
return err
}

// retrieve referenced firmware set attributes
attrs, err := currentValues.FirmwareSetAttributesFirmwareSets().All(ctx, tx)
if err != nil {
return err
}

// remove current referenced firmware set attributes
_, err = attrs.DeleteAll(ctx, tx)
if err != nil {
return err
}

// add new firmware set attributes
if err := newValues.AddFirmwareSetAttributesFirmwareSets(ctx, tx, true, attributes...); err != nil {
return err
// update attributes if newer attributes were given
if len(attrsUpdate) > 0 {
if err := r.firmwareSetAttributesUpdate(ctx, tx, fwSetCurrent, attrsUpdate); err != nil {
return err
}
}

// add new firmware references
// add new firmware references into map
for _, id := range firmwareUUIDs {
m := models.ComponentFirmwareSetMap{FirmwareSetID: newValues.ID, FirmwareID: id.String()}
m := models.ComponentFirmwareSetMap{FirmwareSetID: fwSetUpdate.ID, FirmwareID: id.String()}

err := m.Insert(ctx, tx, boil.Infer())
if err != nil {
Expand All @@ -492,6 +481,34 @@ func (r *Router) firmwareSetUpdateTx(ctx context.Context, newValues *models.Comp
return tx.Commit()
}

func (r *Router) firmwareSetAttributesUpdate(ctx context.Context, tx *sql.Tx, fwSet *models.ComponentFirmwareSet, attrsUpdate models.AttributesFirmwareSetSlice) error {
// retrieve current firmware set attributes
attrsCurrent, err := fwSet.FirmwareSetAttributesFirmwareSets().All(ctx, tx)
if err != nil {
return err
}

// In all cases a single *models.AttributesFirmwareSet obj holds all the firmware set attributes in the Data field,
//
// here we make sure that data is not overwritten by a 'null' or is set to empty.
//
// If the client needs to overwrite this data to be empty, Data should be set to an empty JSON - `{}`
if len(attrsCurrent) == 1 && len(attrsUpdate) == 1 && len(attrsCurrent[0].Data) > 0 {
if string(attrsUpdate[0].Data) == "null" || len(attrsUpdate[0].Data) == 0 {
attrsUpdate[0].Data = attrsCurrent[0].Data
}
}

// remove current referenced firmware set attributes
_, err = attrsCurrent.DeleteAll(ctx, tx)
if err != nil {
return err
}

// add new firmware set attributes
return fwSet.AddFirmwareSetAttributesFirmwareSets(ctx, tx, true, attrsUpdate...)
}

func (r *Router) serverComponentFirmwareSetRemoveFirmware(c *gin.Context) {
firmwareSet, err := r.componentFirmwareSetFromParams(c)
if err != nil {
Expand Down
126 changes: 117 additions & 9 deletions pkg/api/v1/router_firmware_set_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,17 +209,21 @@ func TestIntegrationServerComponentFirmwareSetUpdate(t *testing.T) {
r640FirmwareIDs := r640FirmwareFixtureUUIDs(t, firmware.ComponentFirmware)

var testCases = []struct {
testName string
firmwareSetPayload *serverservice.ComponentFirmwareSetRequest
expectedError bool
expectedResponse string
errorMsg string
testName string
firmwareSetPayload *serverservice.ComponentFirmwareSetRequest
expectFirmwareSetAttributes []serverservice.Attributes
expectedFirmwareCount int
expectedError bool
expectedResponse string
errorMsg string
}{
{
"component firmware set UUID required",
&serverservice.ComponentFirmwareSetRequest{
Name: "foobar",
},
nil,
0,
true,
"400",
"expected a valid firmware set ID, got none",
Expand All @@ -233,6 +237,8 @@ func TestIntegrationServerComponentFirmwareSetUpdate(t *testing.T) {
"d825bbeb-20fb-452e-9fe4-invalid",
},
},
nil,
0,
true,
"400",
"invalid firmware UUID",
Expand All @@ -247,6 +253,8 @@ func TestIntegrationServerComponentFirmwareSetUpdate(t *testing.T) {
r640FirmwareIDs[0],
},
},
nil,
0,
true,
"400",
"exists in firmware set",
Expand All @@ -260,6 +268,8 @@ func TestIntegrationServerComponentFirmwareSetUpdate(t *testing.T) {
"d825bbeb-20fb-452e-9fe4-cdedacb2ca1f",
},
},
nil,
0,
true,
"400",
"firmware object with given UUID does not exist",
Expand All @@ -271,6 +281,8 @@ func TestIntegrationServerComponentFirmwareSetUpdate(t *testing.T) {
ID: firmwareSetID,
ComponentFirmwareUUIDs: []string{dbtools.FixtureDellR640CPLD.ID},
},
nil,
3,
false,
"200",
"",
Expand All @@ -287,6 +299,98 @@ func TestIntegrationServerComponentFirmwareSetUpdate(t *testing.T) {
},
},
},
[]serverservice.Attributes{
{
Namespace: "sh.hollow.firmware_set.labels",
Data: json.RawMessage(`{"created by": "foobar"}`),
},
},
3,
false,
"200",
"",
},
{
"update a firmware set with no attributes data does overwrite existing attributes",
&serverservice.ComponentFirmwareSetRequest{
Name: "foobar-updated",
ID: firmwareSetID,
Attributes: []serverservice.Attributes{
{
Namespace: "sh.hollow.firmware_set.labels",
},
},
},
[]serverservice.Attributes{
{
Namespace: "sh.hollow.firmware_set.labels",
Data: json.RawMessage(`{"created by": "foobar"}`),
},
},
3,
false,
"200",
"",
},
{
"update a firmware set with new attributes updates existing attributes",
&serverservice.ComponentFirmwareSetRequest{
Name: "foobar-updated",
ID: firmwareSetID,
Attributes: []serverservice.Attributes{
{
Namespace: "sh.hollow.firmware_set.labels",
Data: json.RawMessage(`{"updated by": "foo"}`),
},
},
},
[]serverservice.Attributes{
{
Namespace: "sh.hollow.firmware_set.labels",
Data: json.RawMessage(`{"updated by": "foo"}`),
},
},
3,
false,
"200",
"",
},
{
"update an existing firmware set with empty attributes does not purge existing attributes",
&serverservice.ComponentFirmwareSetRequest{
Name: "foobar-updated-again",
ID: firmwareSetID,
},
[]serverservice.Attributes{
{
Namespace: "sh.hollow.firmware_set.labels",
Data: json.RawMessage(`{"updated by": "foo"}`),
},
},
3,
false,
"200",
"",
},
{
"update a firmware set attributes to be empty",
&serverservice.ComponentFirmwareSetRequest{
Name: "foobar-updated",
ID: firmwareSetID,
Attributes: []serverservice.Attributes{
{
Namespace: "sh.hollow.firmware_set.labels",
Data: json.RawMessage(`{}`),
},
},
},
[]serverservice.Attributes{
{
Namespace: "sh.hollow.firmware_set.labels",
Data: json.RawMessage(`{}`),
},
},
3,
false,
"200",
"",
Expand Down Expand Up @@ -315,13 +419,17 @@ func TestIntegrationServerComponentFirmwareSetUpdate(t *testing.T) {
assert.NotNil(t, got)
assert.Equal(t, got.UUID.String(), firmwareSetID.String())
assert.Equal(t, tt.firmwareSetPayload.Name, got.Name)
assert.Equal(t, 3, len(got.ComponentFirmware))
assert.Equal(t, tt.expectedFirmwareCount, len(got.ComponentFirmware))

// assert firmware set attributes
if len(tt.firmwareSetPayload.Attributes) > 0 {
assert.Equal(t, 1, len(got.Attributes))
assert.Equal(t, tt.firmwareSetPayload.Attributes[0].Namespace, got.Attributes[0].Namespace)
assertAttributesEqual(t, tt.firmwareSetPayload.Attributes[0].Data, got.Attributes[0].Data)
}

if tt.expectFirmwareSetAttributes != nil {
assert.NotNil(t, got.Attributes, "Attributes nil")
assertAttributesEqual(t, tt.expectFirmwareSetAttributes[0].Data, got.Attributes[0].Data)
}

if tt.expectedError {
Expand Down Expand Up @@ -431,13 +539,13 @@ func assertAttributesEqual(t *testing.T, a, b []byte) bool {
// unmarshal fixture attribute data
aData := map[string]string{}
if err := json.Unmarshal(a, &aData); err != nil {
t.Fatal(err)
t.Error(err)
}

// unmarshal got attribute data
bData := map[string]string{}
if err := json.Unmarshal(b, &bData); err != nil {
t.Fatal(err)
t.Error(err)
}

return maps.Equal(aData, bData)
Expand Down

0 comments on commit ac2c6eb

Please sign in to comment.