Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion apps/workspace-engine/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"syscall"
"time"

"workspace-engine/pkg/events/handler"
"workspace-engine/pkg/kafka"
"workspace-engine/pkg/server"
"workspace-engine/pkg/ticker"
Expand Down Expand Up @@ -145,14 +146,22 @@ func main() {
defer producer.Close()

// Start periodic ticker for time-sensitive policy evaluation
workspaceTicker := ticker.New(producer)
workspaceTicker := ticker.NewDefault(producer)
go func() {
log.Info("Workspace ticker started")
if err := workspaceTicker.Run(ctx); err != nil {
log.Error("Ticker error", "error", err)
}
}()

workspaceSaveTicker := ticker.New(producer, 1*time.Hour, string(handler.WorkspaceSave))
go func() {
log.Info("Workspace save ticker started")
if err := workspaceSaveTicker.Run(ctx); err != nil {
log.Error("Ticker error", "error", err)
}
}()

go func() {
log.Info("Kafka consumer started")
if err := kafka.RunConsumer(ctx); err != nil {
Expand Down
25 changes: 18 additions & 7 deletions apps/workspace-engine/pkg/ticker/ticker.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,29 @@ var tracer = otel.Tracer("ticker")

// Ticker periodically emits tick events for active workspaces
type Ticker struct {
producer kafka.EventProducer
interval time.Duration
producer kafka.EventProducer
interval time.Duration
eventType string
}

// New creates a new ticker with the configured interval
func New(producer kafka.EventProducer) *Ticker {
// NewDefault creates a new ticker with the configured interval
func NewDefault(producer kafka.EventProducer) *Ticker {
interval := getTickInterval()
log.Info("Ticker initialized", "interval", interval)

return &Ticker{
producer: producer,
interval: interval,
producer: producer,
interval: interval,
eventType: WorkspaceTickEventType,
}
}

// New creates a new ticker with the configured interval and event type
func New(producer kafka.EventProducer, interval time.Duration, eventType string) *Ticker {
return &Ticker{
producer: producer,
interval: interval,
eventType: eventType,
}
}
Comment on lines +47 to 54
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Add input validation to prevent runtime issues.

The constructor should validate that interval is positive and eventType is non-empty to prevent runtime panics (from time.NewTicker) and invalid event production.

Apply this diff to add validation:

 // New creates a new ticker with the configured interval and event type
 func New(producer kafka.EventProducer, interval time.Duration, eventType string) *Ticker {
+	if interval <= 0 {
+		log.Warn("Invalid interval provided, using default", "interval", interval)
+		interval = DefaultTickInterval
+	}
+	if eventType == "" {
+		log.Warn("Empty event type provided, using default")
+		eventType = WorkspaceTickEventType
+	}
+
 	return &Ticker{
 		producer:  producer,
 		interval:  interval,
 		eventType: eventType,
 	}
 }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In apps/workspace-engine/pkg/ticker/ticker.go around lines 47 to 54, validate
inputs in New: ensure interval > 0 and eventType is not empty; if invalid,
return an error (change signature to New(producer kafka.EventProducer, interval
time.Duration, eventType string) (*Ticker, error)), add appropriate fmt.Errorf
messages for each check (or a combined validation error), and only
construct/return the Ticker when validations pass; remember to add the fmt
import and update all call sites to handle the (Ticker, error) return.


Expand Down Expand Up @@ -148,7 +159,7 @@ func (t *Ticker) emitTickForWorkspace(ctx context.Context, workspaceID string) e

span.SetAttributes(attribute.String("workspace.id", workspaceID))

err := t.producer.ProduceEvent(WorkspaceTickEventType, workspaceID, nil)
err := t.producer.ProduceEvent(t.eventType, workspaceID, nil)
if err != nil {
span.RecordError(err)
span.SetStatus(codes.Error, "failed to produce tick event")
Expand Down
20 changes: 12 additions & 8 deletions apps/workspace-engine/pkg/ticker/ticker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@ func (m *MockEventProducer) Close() {
func TestEmitTicks_NoWorkspaces(t *testing.T) {
mockProducer := new(MockEventProducer)
ticker := &Ticker{
producer: mockProducer,
interval: time.Minute,
producer: mockProducer,
interval: time.Minute,
eventType: WorkspaceTickEventType,
}

// Ensure no workspaces are registered
Expand All @@ -49,8 +50,9 @@ func TestEmitTicks_NoWorkspaces(t *testing.T) {
func TestEmitTicks_MultipleWorkspaces(t *testing.T) {
mockProducer := new(MockEventProducer)
ticker := &Ticker{
producer: mockProducer,
interval: time.Minute,
producer: mockProducer,
interval: time.Minute,
eventType: WorkspaceTickEventType,
}

// Register test workspaces
Expand Down Expand Up @@ -80,8 +82,9 @@ func TestEmitTicks_MultipleWorkspaces(t *testing.T) {
func TestEmitTickForWorkspace(t *testing.T) {
mockProducer := new(MockEventProducer)
ticker := &Ticker{
producer: mockProducer,
interval: time.Minute,
producer: mockProducer,
interval: time.Minute,
eventType: WorkspaceTickEventType,
}

workspaceID := "test-workspace"
Expand Down Expand Up @@ -133,8 +136,9 @@ func TestGetTickInterval_Zero(t *testing.T) {
func TestTickerRun_Cancellation(t *testing.T) {
mockProducer := new(MockEventProducer)
ticker := &Ticker{
producer: mockProducer,
interval: 10 * time.Millisecond, // Short interval for testing
producer: mockProducer,
interval: 10 * time.Millisecond, // Short interval for testing
eventType: WorkspaceTickEventType,
}

ctx, cancel := context.WithCancel(context.Background())
Expand Down
Loading