fix: update flavor implementation to enable/disable flavor only

.github/workflows/internal_tests.yaml

@@ -42,7 +42,7 @@ jobs:
           # We exclude packages from coverage analysis as they are highly uncovered at the moment
           EXCLUDE_PKGS="internal/telemetry"
           COVER_PKG=$(go list ./internal/... | grep -Ev "$EXCLUDE_PKGS" | tr '\n' ',')
-          go test -v -coverprofile=coverage.out -race -tags=integration -coverpkg=$COVER_PKG ./internal/...
+          go test -v -count=1 -coverprofile=coverage.out -race -tags=integration -coverpkg=$COVER_PKG ./internal/...
 
       - name: Print coverage report
         run: |
@@ -63,7 +63,6 @@ jobs:
           exit 1
           fi
 
-
     services:
       postgres:
         image: postgres:16.11

internal/database/database.go

@@ -397,50 +397,6 @@ func (d *Database) GetFlavor(ctx context.Context, name string) (*Flavor, error)
 	return &flavor, nil
 }
 
-// UpdateFlavor updates an existing flavor.
-// If the flavor doesn't exist, it will return ErrNotExist.
-func (d *Database) UpdateFlavor(ctx context.Context, flavor *Flavor) error {
-	batch := &pgx.Batch{}
-	batch.Queue(`
-		UPDATE flavor
-		SET 
-			platform = @platform,
-			labels = @labels,
-			priority = @priority,
-			is_disabled = @is_disabled,
-			minimum_pressure = @minimum_pressure
-		WHERE name = @name
-	`, pgx.NamedArgs{
-		"platform":         flavor.Platform,
-		"name":             flavor.Name,
-		"labels":           flavor.Labels,
-		"priority":         flavor.Priority,
-		"is_disabled":      flavor.IsDisabled,
-		"minimum_pressure": flavor.MinimumPressure,
-	})
-	batch.Queue(updateAssignedFlavorSql)
-	batch.Queue(pressureChangeSql)
-	result := d.conn.SendBatch(ctx, batch)
-	defer func() {
-		err := result.Close()
-		if err != nil {
-			slog.ErrorContext(ctx, "cannot close batch execution for updating flavors", "error", err)
-		}
-	}()
-
-	for i := range batch.Len() {
-		tag, err := result.Exec()
-		if err != nil {
-			return fmt.Errorf("cannot update flavor: %w", err)
-		}
-		if i == 0 && tag.RowsAffected() == 0 {
-			return ErrNotExist
-		}
-	}
-
-	return nil
-}
-
 func (d *Database) setFlavorIsDisabled(ctx context.Context, platform, name string, isDisabled bool) error {
 	batch := &pgx.Batch{}
 	if isDisabled {

internal/database/database_test.go

@@ -560,67 +560,6 @@ func TestDatabase_GetFlavor_NotExists(t *testing.T) {
 	assert.Nil(t, flavor)
 }
 
-func TestDatabase_UpdateFlavor(t *testing.T) {
-	/*
-	   arrange: set up database with a flavor
-	   act: update the flavor
-	   assert: flavor in database is updated
-	*/
-	db := setupDatabase(t)
-	defer teardownDatabase(t)
-	ctx := t.Context()
-
-	flavor := Flavor{
-		Platform:        "github",
-		Name:            "amd64-large",
-		Labels:          []string{"self-hosted", "amd64", "large"},
-		Priority:        0,
-		IsDisabled:      false,
-		MinimumPressure: 0,
-	}
-	updatedFlavor := Flavor{
-		Platform:        "github",
-		Name:            "amd64-large",
-		Labels:          []string{"self-hosted", "amd64", "large"},
-		Priority:        10,
-		IsDisabled:      true,
-		MinimumPressure: 5,
-	}
-
-	assert.NoError(t, db.AddFlavor(ctx, &flavor))
-
-	pressureUpdates := subscribeToPressureUpdates(t, ctx, db)
-	assert.NoError(t, db.UpdateFlavor(ctx, &updatedFlavor))
-	assertSingleNotificationReceived(t, pressureUpdates)
-
-	flavors, err := db.ListFlavors(ctx, flavor.Platform)
-	assert.NoError(t, err)
-
-	assert.Len(t, flavors, 1)
-	assert.Equal(t, updatedFlavor, flavors[0])
-}
-
-func TestDatabase_UpdateFlavor_NotExists(t *testing.T) {
-	/*
-	   arrange: set up an empty database
-	   act: update a non-existent flavor
-	   assert: error is raised
-	*/
-	db := setupDatabase(t)
-	defer teardownDatabase(t)
-	ctx := t.Context()
-
-	notExistingFlavor := Flavor{
-		Platform:        "github",
-		Name:            "amd64-large",
-		Labels:          []string{"self-hosted", "amd64", "large"},
-		Priority:        0,
-		IsDisabled:      false,
-		MinimumPressure: 0,
-	}
-	assert.ErrorIs(t, db.UpdateFlavor(ctx, &notExistingFlavor), ErrNotExist)
-}
-
 func TestDatabase_DisableFlavor(t *testing.T) {
 	db := setupDatabase(t)
 	defer teardownDatabase(t)

internal/planner/server.go

@@ -45,7 +45,8 @@ var (
 type FlavorStore interface {
 	AddFlavor(ctx context.Context, flavor *database.Flavor) error
 	ListFlavors(ctx context.Context, platform string) ([]database.Flavor, error)
-	UpdateFlavor(ctx context.Context, flavor *database.Flavor) error
+	EnableFlavor(ctx context.Context, platform, name string) error
+	DisableFlavor(ctx context.Context, platform, name string) error
 	GetFlavor(ctx context.Context, name string) (*database.Flavor, error)
 	DeleteFlavor(ctx context.Context, platform string, name string) error
 	GetPressures(ctx context.Context, platform string, flavors ...string) (map[string]int, error)
@@ -165,33 +166,39 @@ func (s *Server) getFlavor(w http.ResponseWriter, r *http.Request) {
 	respondWithJSON(w, http.StatusOK, flavor)
 }
 
-// updateFlavor handles updating an existing flavor.
-// If the flavor name is allFlavorName, returns status code 400.
-// If the flavor is not found, returns status code 404.
-// If updating the flavor fails, returns status code 500.
-// If successful, returns status code 200.
+type updateFlavorRequest struct {
+	IsDisabled *bool `json:"is_disabled"`
+}
+
+// updateFlavor handles updating an existing flavor's disabled status.
 func (s *Server) updateFlavor(w http.ResponseWriter, r *http.Request) {
-	req, err := decodeFlavorInRequestBody(r)
-	if err != nil {
+	decoder := json.NewDecoder(r.Body)
+	defer r.Body.Close()
+
+	req := &updateFlavorRequest{}
+	if err := decoder.Decode(req); err != nil {
 		http.Error(w, fmt.Sprintf("invalid payload: %v", err), http.StatusBadRequest)
 		return
 	}
 
+	if req.IsDisabled == nil {
+		http.Error(w, "is_disabled field is required", http.StatusBadRequest)
+		return
+	}
+
 	flavorName := r.PathValue("name")
 	if flavorName == allFlavorName {
 		http.Error(w, "update all flavors is not supported", http.StatusBadRequest)
 		return
 	}
-	flavor := &database.Flavor{
-		Name:            flavorName,
-		Platform:        req.Platform,
-		Labels:          req.Labels,
-		Priority:        req.Priority,
-		IsDisabled:      req.IsDisabled,
-		MinimumPressure: req.MinimumPressure,
+
+	var err error
+	if *req.IsDisabled {
+		err = s.store.DisableFlavor(r.Context(), flavorPlatform, flavorName)
+	} else {
+		err = s.store.EnableFlavor(r.Context(), flavorPlatform, flavorName)
 	}
 
-	err = s.store.UpdateFlavor(r.Context(), flavor)
 	if errors.Is(err, database.ErrNotExist) {
 		http.Error(w, "cannot find flavor to update", http.StatusNotFound)
 		return
@@ -200,7 +207,7 @@ func (s *Server) updateFlavor(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, fmt.Sprintf("cannot update flavor: %v", err), http.StatusInternalServerError)
 		return
 	}
-	w.WriteHeader(http.StatusOK)
+	w.WriteHeader(http.StatusNoContent)
 }
 
 // deleteFlavor handles deleting an existing flavor.

internal/planner/server_test.go

@@ -91,13 +91,22 @@ func (f *fakeStore) GetFlavor(ctx context.Context, name string) (*database.Flavo
 	return nil, f.errToReturn
 }
 
-func (f *fakeStore) UpdateFlavor(ctx context.Context, flavor *database.Flavor) error {
-	f.lastFlavor = flavor
+func (f *fakeStore) DeleteFlavor(ctx context.Context, platform, name string) error {
+	f.lastFlavor = nil
 	return f.errToReturn
 }
 
-func (f *fakeStore) DeleteFlavor(ctx context.Context, platform, name string) error {
-	f.lastFlavor = nil
+func (f *fakeStore) EnableFlavor(ctx context.Context, platform, name string) error {
+	if f.lastFlavor != nil && f.lastFlavor.Name == name {
+		f.lastFlavor.IsDisabled = false
+	}
+	return f.errToReturn
+}
+
+func (f *fakeStore) DisableFlavor(ctx context.Context, platform, name string) error {
+	if f.lastFlavor != nil && f.lastFlavor.Name == name {
+		f.lastFlavor.IsDisabled = true
+	}
 	return f.errToReturn
 }
 
@@ -352,15 +361,15 @@ func TestUpdateFlavor(t *testing.T) {
 		assert: Verify the flavors are updated, and/or the HTTP status codes are as expected.
 	*/
 	tests := []struct {
-		name           string
-		storeFlavor    *database.Flavor
-		storeErr       error
-		url            string
-		body           string
-		expectedStatus int
-		assertFlavor   bool
+		name             string
+		storeFlavor      *database.Flavor
+		storeErr         error
+		url              string
+		body             string
+		expectedStatus   int
+		expectedDisabled bool
 	}{{
-		name: "shouldSucceed",
+		name: "shouldDisableFlavor",
 		storeFlavor: &database.Flavor{
 			Platform:        "github",
 			Name:            "runner-small",
@@ -369,24 +378,36 @@ func TestUpdateFlavor(t *testing.T) {
 			IsDisabled:      false,
 			MinimumPressure: 5,
 		},
-		url:            "/api/v1/flavors/runner-small",
-		body:           `{"platform":"github","name":"runner-small","labels":["x64"],"priority":5,"is_disabled":false,"minimum_pressure":10}`,
-		expectedStatus: http.StatusOK,
-		assertFlavor:   true,
+		url:              "/api/v1/flavors/runner-small",
+		body:             `{"is_disabled":true}`,
+		expectedStatus:   http.StatusNoContent,
+		expectedDisabled: true,
+	}, {
+		name: "shouldEnableFlavor",
+		storeFlavor: &database.Flavor{
+			Platform:        "github",
+			Name:            "runner-small",
+			Labels:          []string{"x64"},
+			Priority:        10,
+			IsDisabled:      true,
+			MinimumPressure: 5,
+		},
+		url:              "/api/v1/flavors/runner-small",
+		body:             `{"is_disabled":false}`,
+		expectedStatus:   http.StatusNoContent,
+		expectedDisabled: false,
 	}, {
 		name:           "shouldFailWhenFlavorMissing",
 		storeErr:       database.ErrNotExist,
 		url:            "/api/v1/flavors/not-exist",
-		body:           `{"platform":"github","name":"not-exist","labels":["x64"],"priority":5,"is_disabled":false,"minimum_pressure":10}`,
+		body:           `{"is_disabled":true}`,
 		expectedStatus: http.StatusNotFound,
-		assertFlavor:   false,
 	}, {
 		name:           "shouldFailWhenDatabaseError",
 		storeErr:       errors.New("database error"),
 		url:            "/api/v1/flavors/runner-small",
-		body:           `{"platform":"github","name":"runner-small","labels":["x64"],"priority":5,"is_disabled":false,"minimum_pressure":10}`,
+		body:           `{"is_disabled":false}`,
 		expectedStatus: http.StatusInternalServerError,
-		assertFlavor:   false,
 	}, {
 		name: "shouldFailWhenNameIsAllFlavors",
 		storeFlavor: &database.Flavor{
@@ -398,7 +419,7 @@ func TestUpdateFlavor(t *testing.T) {
 			MinimumPressure: 5,
 		},
 		url:            "/api/v1/flavors/_",
-		body:           `{"platform":"github","name":"not-exist","labels":["x64"],"priority":5,"is_disabled":false,"minimum_pressure":10}`,
+		body:           `{"is_disabled":true}`,
 		expectedStatus: http.StatusBadRequest,
 	}, {
 		name: "shouldFailOnInvalidJSON",
@@ -413,6 +434,19 @@ func TestUpdateFlavor(t *testing.T) {
 		url:            "/api/v1/flavors/runner-small",
 		body:           `{invalid-json}`,
 		expectedStatus: http.StatusBadRequest,
+	}, {
+		name: "shouldFailWhenIsDisabledFieldMissing",
+		storeFlavor: &database.Flavor{
+			Platform:        "github",
+			Name:            "runner-small",
+			Labels:          []string{"x64"},
+			Priority:        10,
+			IsDisabled:      false,
+			MinimumPressure: 5,
+		},
+		url:            "/api/v1/flavors/runner-small",
+		body:           `{}`,
+		expectedStatus: http.StatusBadRequest,
 	}}
 
 	for _, tt := range tests {
@@ -427,12 +461,10 @@ func TestUpdateFlavor(t *testing.T) {
 			server.ServeHTTP(w, req)
 
 			assert.Equal(t, tt.expectedStatus, w.Code)
-			if tt.assertFlavor {
-				flavor := &database.Flavor{}
-				require.NoError(t, json.Unmarshal([]byte(tt.body), flavor))
-				storedFlavor, err := store.GetFlavor(t.Context(), flavor.Name)
+			if tt.expectedStatus == http.StatusNoContent {
+				storedFlavor, err := store.GetFlavor(t.Context(), tt.storeFlavor.Name)
 				require.NoError(t, err)
-				assert.Equal(t, flavor, storedFlavor)
+				assert.Equal(t, tt.expectedDisabled, storedFlavor.IsDisabled)
 			}
 		})
 	}

internal/planner/telemetry_test.go

@@ -46,11 +46,19 @@ func (m *mockStore) ListFlavors(ctx context.Context, platform string) ([]databas
 func (m *mockStore) GetFlavor(ctx context.Context, name string) (*database.Flavor, error) {
 	return m.lastFlavor, nil
 }
-func (m *mockStore) UpdateFlavor(ctx context.Context, flavor *database.Flavor) error {
-	m.lastFlavor = flavor
+func (m *mockStore) DeleteFlavor(ctx context.Context, platform, name string) error {
 	return nil
 }
-func (m *mockStore) DeleteFlavor(ctx context.Context, platform, name string) error {
+func (m *mockStore) EnableFlavor(ctx context.Context, platform, name string) error {
+	if m.lastFlavor != nil && m.lastFlavor.Name == name {
+		m.lastFlavor.IsDisabled = false
+	}
+	return nil
+}
+func (m *mockStore) DisableFlavor(ctx context.Context, platform, name string) error {
+	if m.lastFlavor != nil && m.lastFlavor.Name == name {
+		m.lastFlavor.IsDisabled = true
+	}
 	return nil
 }
 func (m *mockStore) GetPressures(ctx context.Context, platform string, flavors ...string) (map[string]int, error) {