Du kannst nicht mehr als 25 Themen auswählen Themen müssen entweder mit einem Buchstaben oder einer Ziffer beginnen. Sie können Bindestriche („-“) enthalten und bis zu 35 Zeichen lang sein.
 
 
 
 

11 KiB

Code Review: AFASystems Presence Detection System

Date: 2026-01-15 Reviewer: Claude Code Project: BLE Beacon Presence Detection System


Overall Assessment

Your system is a well-structured microservices architecture for BLE beacon presence detection. The code is functional but has several areas that need refactoring for production readiness, maintainability, and robustness.

Code Quality Score: 6.5/10

  • Good architecture and separation
  • Thread-safe concurrent access
  • No testing
  • Poor error handling
  • Security concerns
  • Code duplication

🔴 Critical Issues (Fix Immediately)

1. Hardcoded Credentials in Config

Location: internal/pkg/config/config.go:46-49 Risk: Security vulnerability - default credentials exposed in source code

ClientSecret: getEnv("ClientSecret", "wojuoB7Z5xhlPFrF2lIxJSSdVHCApEgC"),
HTTPPassword: getEnv("HTTPPassword", "C0r3_us3r_Cr3d3nt14ls"),

Fix: Remove default credentials, require explicit environment configuration:

ClientSecret: getEnvOrFatal("ClientSecret"),  // Helper that panics if not set
HTTPPassword: getEnvOrFatal("HTTPPassword"),

2. Global Database Variable

Location: internal/pkg/database/database.go:12

var DB *gorm.DB  // ❌ Global variable

Issues:

  • Hard to test
  • Implicit dependencies
  • Cannot have multiple DB connections

Fix: Return *gorm.DB from Connect() and inject it into services.

3. Missing Error Context

Errors are logged but lose context:

fmt.Println("Error in sending Kafka message:", err)  // ❌ No context

Fix: Use structured logging:

slog.Error("failed to send kafka message",
    "topic", topic,
    "beacon_id", id,
    "error", err)

4. Unsafe Map Access

Location: cmd/bridge/main.go:28

hostname := strings.Split(topic, "/")[1]  // ❌ Panic if index doesn't exist

Fix: Validate before accessing:

parts := strings.Split(topic, "/")
if len(parts) < 2 {
    slog.Warn("invalid topic format", "topic", topic)
    return
}
hostname := parts[1]

🟡 High Priority Refactoring (Fix Soon)

5. Code Duplication Across Services

Affected Files: All 4 main files

Pattern: Logging setup (lines 124-131 in all services)

logFile, err := os.OpenFile("server.log", os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666)
w := io.MultiWriter(os.Stderr, logFile)
logger := slog.New(slog.NewJSONHandler(w, nil))
slog.SetDefault(logger)

Refactor: Create internal/pkg/common/logger/logger.go:

func SetupLogger(filename string) *slog.Logger {
    logFile, err := os.OpenFile(filename, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666)
    if err != nil {
        log.Fatalf("Failed to open log file: %v\n", err)
    }
    w := io.MultiWriter(os.Stderr, logFile)
    logger := slog.New(slog.NewJSONHandler(w, nil))
    slog.SetDefault(logger)
    return logger
}

6. Inconsistent Error Handling

Mixed error handling patterns across codebase:

In controller: internal/pkg/controller/trackers_controller.go:48

if err != nil {
    fmt.Println("error in sending Kafka POST message")
    http.Error(w, "Error in sending kafka message", 500)
    return
}

In main: cmd/decoder/main.go:97

if err != nil {
    eMsg := fmt.Sprintf("Error in decoding: %v", err)
    fmt.Println(eMsg)
    return
}

Fix: Use consistent error handling with structured responses.

7. Missing Configuration Validation

Location: internal/pkg/config/config.go

Config doesn’t validate required fields:

func Load() *Config {
    return &Config{
        // No validation
    }
}

Fix: Add validation:

func (c *Config) Validate() error {
    if c.DBHost == "" {
        return errors.New("DBHost is required")
    }
    if c.KafkaURL == "" {
        return errors.New("KafkaURL is required")
    }
    // ... other validations
    return nil
}

8. Potential Memory Inefficiency

Location: cmd/location/main.go:217-222

if len(beacon.BeaconMetrics) >= settings.BeaconMetricSize {
    copy(beacon.BeaconMetrics, beacon.BeaconMetrics[1:])
    beacon.BeaconMetrics[settings.BeaconMetricSize-1] = metric
} else {
    beacon.BeaconMetrics = append(beacon.BeaconMetrics, metric)
}

Issue: This logic is correct but could be optimized with a circular buffer.


🟢 Medium Priority Improvements (Plan Refactor)

9. Tight Coupling in Controllers

Controllers directly use *gorm.DB instead of repository pattern:

func TrackerAdd(db *gorm.DB, writer *kafka.Writer, ctx context.Context) http.HandlerFunc

Refactor: Introduce repository interface:

type TrackerRepository interface {
    Create(tracker *model.Tracker) error
    Find(id string) (*model.Tracker, error)
    // ...
}

func TrackerAdd(repo TrackerRepository, writer *kafka.Writer, ctx context.Context) http.HandlerFunc

10. Poor Separation of Concerns

Location: internal/pkg/common/appcontext/context.go

The AppState mixes state management with Kafka client creation (lines 60-76).

Refactor: Separate concerns:

// internal/pkg/infrastructure/kafka/pool.go
type KafkaPool struct {
    writers []*kafka.Writer
    readers []*kafka.Reader
}

// internal/pkg/domain/appstate/state.go
type AppState struct {
    beacons   *model.BeaconsList
    settings  *model.Settings
    kafkaPool *kafka.KafkaPool  // Composed, not owned
}

11. Magic Numbers

Location: cmd/location/main.go:107-108

seenW := 1.5  // What does this mean?
rssiW := 0.75 // What does this mean?

Fix: Extract to named constants in settings:

type SettingsVal struct {
    LocationConfidence int64   `json:"location_confidence"`
    SeenWeight        float64 `json:"seen_weight"`      // 1.5
    RSSIWeight        float64 `json:"rssi_weight"`      // 0.75
    // ...
}

12. Inefficient JSON Marshaling

Location: cmd/server/main.go:206-207

js, err := json.Marshal(list)
if err != nil {
    js = []byte("error")  // ❌ Invalid JSON!
}

Fix: Return proper error response:

if err != nil {
    http.Error(w, "Failed to marshal trackers", http.StatusInternalServerError)
    return
}

🔵 Low Priority / Technical Debt

13. Zero Test Coverage

The codebase has no test files. This is critical for production systems.

Recommendation: Add unit tests for:

  • All business logic in service/ package
  • Controller handlers
  • Kafka message processing
  • Beacon parsing logic

Target: 70%+ code coverage

14. Missing Context Timeouts

Location: cmd/bridge/main.go:68

err = writer.WriteMessages(context.Background(), msg)

Fix: Use timeouts:

ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
err = writer.WriteMessages(ctx, msg)

15. Inefficient String Concatenation

Location: cmd/bridge/main.go:182

lMsg := fmt.Sprintf("Beacon added to lookup: %s", id)
slog.Info(lMsg)

Fix: Direct logging:

slog.Info("beacon added to lookup", "id", id)

16. Dead Code

Location: cmd/bridge/main.go:76-103

Large block of commented code should be removed.

17. Incomplete Graceful Shutdown

Location: cmd/bridge/main.go:212

The MQTT client disconnects with timeout but doesn’t wait for pending messages:

client.Disconnect(250)  // Only waits 250ms

18. No Health Checks

Services don’t expose health endpoints for orchestration systems (Kubernetes, etc.).


📊 Architecture Recommendations

1. Implement Dependency Injection

Instead of passing db, writer, ctx to controllers, create a service container:

type Services struct {
    DB          *gorm.DB
    KafkaWriter *kafka.Writer
    AppState    *appcontext.AppState
}

func (s *Services) TrackerAddController() http.HandlerFunc {
    return func(w http.ResponseWriter, r *http.Request) {
        // Use s.DB, s.KafkaWriter
    }
}

2. Add Observability

  • Structured logging with request IDs
  • Metrics (Prometheus) for:
    • Kafka message throughput
    • Beacon processing latency
    • Database query performance
  • Distributed tracing (OpenTelemetry)

3. Implement Circuit Breakers

For external API calls in apiclient package to handle failures gracefully.

4. Add Message Validation

Validate Kafka messages before processing:

func (adv *BeaconAdvertisement) Validate() error {
    if adv.ID == "" {
        return errors.New("beacon ID is required")
    }
    if adv.RSSI < -100 || adv.RSSI > 0 {
        return errors.New("invalid RSSI value")
    }
    return nil
}

🎯 Refactoring Priority Order

Priority Category Actions
1 🔴 Security Remove hardcoded credentials from config
2 🔴 Stability Fix unsafe map/array access
3 🔴 Testing Add unit tests (aim for 70%+ coverage)
4 🟡 Error Handling Implement structured error handling
5 🟡 Logging Standardize to structured logging throughout
6 🟡 Code Quality Extract duplicated code to shared packages
7 🟢 Architecture Implement dependency injection gradually
8 🔵 Performance Optimize hot paths (beacon processing)

📈 Metrics Summary

Category Count Status
🔴 Critical Issues 4 Fix Immediately
🟡 High Priority 4 Fix Soon
🟢 Medium Priority 4 Plan Refactor
🔵 Low Priority 6 Technical Debt

Total Issues Identified: 18


System Architecture Overview

The system consists of 4 microservices:

  1. Bridge (cmd/bridge/main.go) - MQTT to Kafka bridge
  2. Decoder (cmd/decoder/main.go) - BLE beacon decoder
  3. Location (cmd/location/main.go) - Location calculation service
  4. Server (cmd/server/main.go) - HTTP API & WebSocket server

Communication Flow

MQTT Gateway → Bridge (Kafka) → Decoder (Kafka) → Location (Kafka) → Server (Kafka)
                ↓                                                      ↑
           External API ←───────────────────────────────────────────────

Technology Stack

  • Language: Go 1.24.0
  • Message Broker: Apache Kafka
  • Database: PostgreSQL with GORM
  • Cache: Redis (valkey)
  • MQTT: Eclipse Paho
  • HTTP: Gorilla Mux + WebSocket
  • Deployment: Docker Compose

Conclusion

The codebase demonstrates solid understanding of microservices architecture with good separation of concerns. The concurrent access patterns using sync.RWMutex are well-implemented. However, the system needs significant hardening before production deployment, particularly in areas of security, testing, and error handling.

Focus on addressing critical security issues first, then build out test coverage to ensure reliability as you refactor other areas of the codebase.