Merge pull request #124 from kuudori/HYPERFLEET-995

HYPERFLEET-995 - refactor: move  logic from handlers to service

Author: openshift-merge-bot[bot]

pkg/api/adapter_status_types.go

@@ -46,11 +46,13 @@ func (l AdapterStatusList) Index() AdapterStatusIndex {
 }
 
 func (as *AdapterStatus) BeforeCreate(tx *gorm.DB) error {
-	id, err := NewID()
-	if err != nil {
-		return fmt.Errorf("failed to generate adapter status ID: %w", err)
+	if as.ID == "" {
+		id, err := NewID()
+		if err != nil {
+			return fmt.Errorf("failed to generate adapter status ID: %w", err)
+		}
+		as.ID = id
 	}
-	as.ID = id
 	return nil
 }
 

pkg/api/cluster_types.go

@@ -24,8 +24,10 @@ type Cluster struct {
 	Generation       int32          `json:"generation" gorm:"default:1;not null"`
 }
 
-type ClusterList []*Cluster
-type ClusterIndex map[string]*Cluster
+type (
+	ClusterList  []*Cluster
+	ClusterIndex map[string]*Cluster
+)
 
 func (l ClusterList) Index() ClusterIndex {
 	index := ClusterIndex{}
@@ -36,14 +38,18 @@ func (l ClusterList) Index() ClusterIndex {
 }
 
 func (c *Cluster) BeforeCreate(tx *gorm.DB) error {
-	id, err := NewID()
-	if err != nil {
-		return fmt.Errorf("failed to generate cluster ID: %w", err)
+	if c.ID == "" {
+		id, err := NewID()
+		if err != nil {
+			return fmt.Errorf("failed to generate cluster ID: %w", err)
+		}
+		c.ID = id
 	}
-	c.ID = id
 
 	now := time.Now()
-	c.CreatedTime = now
+	if c.CreatedTime.IsZero() {
+		c.CreatedTime = now
+	}
 	c.UpdatedTime = now
 	if c.Generation == 0 {
 		c.Generation = 1
@@ -64,3 +70,12 @@ type ClusterPatchRequest struct {
 	Spec   *map[string]interface{} `json:"spec,omitempty"`
 	Labels *map[string]string      `json:"labels,omitempty"`
 }
+
+func (c *Cluster) MarkDeleted(by string, t time.Time) {
+	c.DeletedTime = &t
+	c.DeletedBy = &by
+}
+
+func (c *Cluster) IncrementGeneration() {
+	c.Generation++
+}

pkg/api/node_pool_types.go

@@ -28,8 +28,10 @@ type NodePool struct {
 	Generation       int32          `json:"generation" gorm:"default:1;not null"`
 }
 
-type NodePoolList []*NodePool
-type NodePoolIndex map[string]*NodePool
+type (
+	NodePoolList  []*NodePool
+	NodePoolIndex map[string]*NodePool
+)
 
 func (l NodePoolList) Index() NodePoolIndex {
 	index := NodePoolIndex{}
@@ -40,14 +42,18 @@ func (l NodePoolList) Index() NodePoolIndex {
 }
 
 func (np *NodePool) BeforeCreate(tx *gorm.DB) error {
-	id, err := NewID()
-	if err != nil {
-		return fmt.Errorf("failed to generate node pool ID: %w", err)
+	if np.ID == "" {
+		id, err := NewID()
+		if err != nil {
+			return fmt.Errorf("failed to generate node pool ID: %w", err)
+		}
+		np.ID = id
 	}
-	np.ID = id
 
 	now := time.Now()
-	np.CreatedTime = now
+	if np.CreatedTime.IsZero() {
+		np.CreatedTime = now
+	}
 	np.UpdatedTime = now
 	if np.Generation == 0 {
 		np.Generation = 1
@@ -75,3 +81,12 @@ type NodePoolPatchRequest struct {
 	Spec   *map[string]interface{} `json:"spec,omitempty"`
 	Labels *map[string]string      `json:"labels,omitempty"`
 }
+
+func (np *NodePool) MarkDeleted(by string, t time.Time) {
+	np.DeletedTime = &t
+	np.DeletedBy = &by
+}
+
+func (np *NodePool) IncrementGeneration() {
+	np.Generation++
+}

pkg/api/presenters/cluster.go

@@ -11,7 +11,7 @@ import (
 )
 
 // ConvertCluster converts openapi.ClusterCreateRequest to api.Cluster (GORM model)
-func ConvertCluster(req *openapi.ClusterCreateRequest, createdBy string) (*api.Cluster, error) {
+func ConvertCluster(req *openapi.ClusterCreateRequest) (*api.Cluster, error) {
 	// Marshal Spec
 	specJSON, err := json.Marshal(req.Spec)
 	if err != nil {
@@ -40,8 +40,6 @@ func ConvertCluster(req *openapi.ClusterCreateRequest, createdBy string) (*api.C
 		Spec:       specJSON,
 		Labels:     labelsJSON,
 		Generation: 1,
-		CreatedBy:  createdBy,
-		UpdatedBy:  createdBy,
 	}, nil
 }
 

pkg/api/presenters/cluster_test.go

@@ -36,16 +36,13 @@ func TestConvertCluster_Complete(t *testing.T) {
 	RegisterTestingT(t)
 
 	req := createTestClusterRequest()
-	createdBy := "user123"
 
-	result, err := ConvertCluster(req, createdBy)
+	result, err := ConvertCluster(req)
 	Expect(err).To(BeNil())
 
 	// Verify basic fields
 	Expect(result.Kind).To(Equal("Cluster"))
 	Expect(result.Name).To(Equal("test-cluster"))
-	Expect(result.CreatedBy).To(Equal(createdBy))
-	Expect(result.UpdatedBy).To(Equal(createdBy))
 
 	// Verify defaults
 	Expect(result.Generation).To(Equal(int32(1)))
@@ -83,7 +80,7 @@ func TestConvertCluster_WithLabels(t *testing.T) {
 		Spec:   map[string]interface{}{"test": "spec"},
 	}
 
-	result, err := ConvertCluster(req, "user456")
+	result, err := ConvertCluster(req)
 	Expect(err).To(BeNil())
 
 	var resultLabels map[string]string
@@ -104,7 +101,7 @@ func TestConvertCluster_WithoutLabels(t *testing.T) {
 		Spec:   map[string]interface{}{"test": "spec"},
 	}
 
-	result, err := ConvertCluster(req, "user789")
+	result, err := ConvertCluster(req)
 	Expect(err).To(BeNil())
 
 	var resultLabels map[string]string
@@ -135,7 +132,7 @@ func TestConvertCluster_SpecMarshaling(t *testing.T) {
 		Spec: complexSpec,
 	}
 
-	result, err := ConvertCluster(req, "user000")
+	result, err := ConvertCluster(req)
 	Expect(err).To(BeNil())
 
 	var resultSpec map[string]interface{}
@@ -319,13 +316,12 @@ func TestConvertAndPresentCluster_RoundTrip(t *testing.T) {
 	RegisterTestingT(t)
 
 	originalReq := createTestClusterRequest()
-	createdBy := "user999@example.com"
 
 	// Convert from OpenAPI request to domain
-	cluster, err := ConvertCluster(originalReq, createdBy)
+	cluster, err := ConvertCluster(originalReq)
 	Expect(err).To(BeNil())
 
-	// Simulate database fields (ID, timestamps)
+	// Simulate database fields (ID, timestamps, created_by/updated_by set by service layer)
 	cluster.ID = "cluster-roundtrip-123"
 	now := time.Now()
 	cluster.CreatedTime = now
@@ -339,8 +335,6 @@ func TestConvertAndPresentCluster_RoundTrip(t *testing.T) {
 	Expect(*result.Id).To(Equal("cluster-roundtrip-123"))
 	Expect(result.Kind).To(Equal(originalReq.Kind))
 	Expect(result.Name).To(Equal(originalReq.Name))
-	Expect(result.CreatedBy).To(Equal(openapi_types.Email(createdBy)))
-	Expect(result.UpdatedBy).To(Equal(openapi_types.Email(createdBy)))
 
 	// Verify Spec preserved
 	Expect(result.Spec["region"]).To(Equal(originalReq.Spec["region"]))

pkg/api/presenters/node_pool.go

@@ -9,7 +9,7 @@ import (
 )
 
 // ConvertNodePool converts openapi.NodePoolCreateRequest to api.NodePool (GORM model)
-func ConvertNodePool(req *openapi.NodePoolCreateRequest, ownerID, createdBy string) (*api.NodePool, error) {
+func ConvertNodePool(req *openapi.NodePoolCreateRequest, ownerID string) (*api.NodePool, error) {
 	// Marshal Spec
 	specJSON, err := json.Marshal(req.Spec)
 	if err != nil {
@@ -38,8 +38,6 @@ func ConvertNodePool(req *openapi.NodePoolCreateRequest, ownerID, createdBy stri
 		Labels:    labelsJSON,
 		OwnerID:   ownerID,
 		OwnerKind: "Cluster",
-		CreatedBy: createdBy,
-		UpdatedBy: createdBy,
 	}, nil
 }
 

pkg/api/presenters/node_pool_test.go

@@ -33,18 +33,15 @@ func TestConvertNodePool_Complete(t *testing.T) {
 
 	req := createTestNodePoolRequest()
 	ownerID := "cluster-owner-123"
-	createdBy := "user456"
 
-	result, err := ConvertNodePool(req, ownerID, createdBy)
+	result, err := ConvertNodePool(req, ownerID)
 	Expect(err).To(BeNil())
 
 	// Verify basic fields
 	Expect(result.Kind).To(Equal("NodePool"))
 	Expect(result.Name).To(Equal("test-nodepool"))
 	Expect(result.OwnerID).To(Equal("cluster-owner-123"))
 	Expect(result.OwnerKind).To(Equal("Cluster"))
-	Expect(result.CreatedBy).To(Equal("user456"))
-	Expect(result.UpdatedBy).To(Equal("user456"))
 
 	// Verify Spec marshaled correctly
 	var spec map[string]interface{}
@@ -75,7 +72,7 @@ func TestConvertNodePool_WithKind(t *testing.T) {
 		Labels: nil,
 	}
 
-	result, err := ConvertNodePool(req, "cluster-123", "user789")
+	result, err := ConvertNodePool(req, "cluster-123")
 	Expect(err).To(BeNil())
 
 	Expect(result.Kind).To(Equal("CustomNodePool"))
@@ -92,7 +89,7 @@ func TestConvertNodePool_WithoutKind(t *testing.T) {
 		Labels: nil,
 	}
 
-	result, err := ConvertNodePool(req, "cluster-456", "user000")
+	result, err := ConvertNodePool(req, "cluster-456")
 	Expect(err).To(BeNil())
 
 	Expect(result.Kind).To(Equal("NodePool")) // Default value
@@ -114,7 +111,7 @@ func TestConvertNodePool_WithLabels(t *testing.T) {
 		Labels: &labels,
 	}
 
-	result, err := ConvertNodePool(req, "cluster-789", "user111")
+	result, err := ConvertNodePool(req, "cluster-789")
 	Expect(err).To(BeNil())
 
 	var resultLabels map[string]string
@@ -135,7 +132,7 @@ func TestConvertNodePool_WithoutLabels(t *testing.T) {
 		Labels: nil, // Nil labels
 	}
 
-	result, err := ConvertNodePool(req, "cluster-xyz", "user222")
+	result, err := ConvertNodePool(req, "cluster-xyz")
 	Expect(err).To(BeNil())
 
 	var resultLabels map[string]string
@@ -361,10 +358,9 @@ func TestConvertAndPresentNodePool_RoundTrip(t *testing.T) {
 
 	originalReq := createTestNodePoolRequest()
 	ownerID := "cluster-roundtrip-789"
-	createdBy := "user-roundtrip@example.com"
 
 	// Convert from OpenAPI request to domain
-	nodePool, err := ConvertNodePool(originalReq, ownerID, createdBy)
+	nodePool, err := ConvertNodePool(originalReq, ownerID)
 	Expect(err).To(BeNil())
 
 	// Simulate database fields (ID, timestamps)
@@ -381,8 +377,6 @@ func TestConvertAndPresentNodePool_RoundTrip(t *testing.T) {
 	Expect(*result.Id).To(Equal("nodepool-roundtrip-123"))
 	Expect(*result.Kind).To(Equal(*originalReq.Kind))
 	Expect(result.Name).To(Equal(originalReq.Name))
-	Expect(result.CreatedBy).To(Equal(openapi_types.Email(createdBy)))
-	Expect(result.UpdatedBy).To(Equal(openapi_types.Email(createdBy)))
 
 	// Verify Spec preserved
 	Expect(result.Spec["replicas"]).To(BeNumerically("==", originalReq.Spec["replicas"]))

pkg/dao/CLAUDE.md

@@ -30,7 +30,7 @@ This is critical — without it, the middleware will commit a partially-failed t
 
 ## Patterns
 
-- `Replace()` compares spec bytes to detect changes; auto-increments `Generation` only when spec actually changed
+- Generation increment is handled by the service layer's `Patch` method via `IncrementGeneration()`; `Save()` persists the result
 - Use `clause.Associations` carefully — omit on updates to prevent cascading deletes of related records
 - All methods accept `context.Context` as first parameter for transaction propagation
 - Return stdlib `error` (not `*errors.ServiceError`) — service layer wraps DAO errors into ServiceErrors