Merge pull request #10 from IFRCGo/fix/consistency-improvements

Refactor: improve overall consistency

Author: frozenhelium

.github/workflows/ci.yml

@@ -43,6 +43,9 @@ jobs:
       - name: Vet
         run: go vet ./...
 
+      - name: Test
+        run: go test ./...
+
       - name: Build
         run: go build -trimpath -ldflags="-s -w" -o cacheppuccino .
 
@@ -84,13 +87,31 @@ jobs:
             type=sha,format=short,prefix=0.1.0-c
             type=ref,event=tag
 
+      - name: Determine build version
+        id: ver
+        shell: bash
+        run: |
+          set -euo pipefail
+
+          if [[ "${GITHUB_REF_TYPE}" == "tag" ]]; then
+            # Tag format: v0.1.0 -> 0.1.0
+            VERSION="${GITHUB_REF_NAME#v}"
+          else
+            SHORT_SHA="$(echo "${GITHUB_SHA}" | cut -c1-7)"
+            VERSION="0.1.0-c${SHORT_SHA}"
+          fi
+
+          echo "version=${VERSION}" >> "$GITHUB_OUTPUT"
+
       - name: Build and push docker image
         uses: docker/build-push-action@v6
         with:
           context: .
           push: true
           tags: ${{ steps.meta.outputs.tags }}
           labels: ${{ steps.meta.outputs.labels }}
+          build-args: |
+            VERSION=${{ steps.ver.outputs.version }}
           cache-from: type=gha
           cache-to: type=gha,mode=max
   helm:

Dockerfile

@@ -1,6 +1,8 @@
 # -------- build stage --------
 FROM golang:1.23 AS build
 
+ARG VERSION=dev
+
 ENV CGO_ENABLED=0 \
     GOOS=linux \
     GOARCH=amd64
@@ -12,7 +14,7 @@ RUN go mod download
 
 COPY . .
 
-RUN go build -trimpath -ldflags="-s -w" -o /out/cacheppuccino .
+RUN go build -trimpath -ldflags="-s -w -X main.version=${VERSION}" -o /out/cacheppuccino .
 
 # -------- runtime stage --------
 

README.md

@@ -7,9 +7,10 @@ It periodically downloads an XLSX export from a translation service, stores the
 ## Features
 
 - XLSX import from external translation service
-- Periodic background sync
+- Periodic background sync (full replace per import; the XLSX is the source of truth)
 - SQLite-backed cache
 - Fetch translations by page(s) + language
+- ETag / If-None-Match support on `/strings`
 - Consistent JSON response envelope
 - OpenAPI 3 schema generation (via kin-openapi)
 - Health + readiness endpoints
@@ -81,11 +82,24 @@ curl "http://localhost:8080/strings?pages=home,about&lang=en"
 
 On startup:
 
-1. Performs an initial XLSX pull.
-2. If successful → service becomes ready.
+1. If the SQLite cache already holds data from a previous run, `/status` reports `ready: true` immediately.
+2. Performs an initial XLSX pull. A successful pull also sets `ready: true`.
 3. Periodically refreshes based on `PULL_INTERVAL`.
 
-The service avoids re-importing unchanged XLSX files by hashing the downloaded content.
+Each import fully replaces the cached strings inside a single transaction, so rows removed
+from the XLSX disappear from the cache. The service avoids re-importing unchanged XLSX
+files by hashing the downloaded content.
+
+### XLSX format
+
+The export must contain a sheet named `Translations` (falls back to the first sheet), with
+a header row of exactly `page | key | <lang> | <lang> | ...` (case-insensitive). Files with
+other header names (e.g. `Namespace`) are rejected.
+
+### Response caching
+
+`/strings` responses carry an `ETag` derived from the last import hash and
+`Cache-Control: public, max-age=60`. Requests with a matching `If-None-Match` get `304 Not Modified`.
 
 
 ## Environment Variables
@@ -94,7 +108,7 @@ The service avoids re-importing unchanged XLSX files by hashing the downloaded c
 |----------|----------|------------|
 | `TRANSLATION_BASE_URL` | Yes | Base URL of translation service |
 | `TRANSLATION_APPLICATION_ID` | Yes | Translation application ID |
-| `TRANSLATION_API_KEY` | No | Sent as `X-API-KEY` header |
+| `TRANSLATION_API_KEY` | Yes | Sent as `X-API-KEY` header |
 | `SQLITE_PATH` | No | Default: `/data/cacheppuccino.db` |
 | `PULL_INTERVAL` | No | Default: `10m` |
 | `HTTP_TIMEOUT` | No | Default: `30s` |
@@ -135,7 +149,10 @@ docker compose down -v
 ## Health Checks
 
 - `GET /healthz` → service running
-- `GET /readyz` → initial sync completed
+- `GET /readyz` → always `200` while the process is up. Deliberately not gated on data:
+  the deploy tooling restarts pods that stay unready, and with a single replica there is
+  no alternative pod to route to. Whether the cache actually holds servable data is
+  reported as `ready` on `GET /status`.
 - Docker healthcheck uses internal `--healthcheck` flag
 
 
@@ -159,8 +176,8 @@ go run . --schema
 ## Database
 
 - SQLite
-- WAL mode enabled
-- Uses `mode=rwc`
+- WAL mode with `synchronous=NORMAL`
+- Single connection (`MaxOpenConns=1`)
 - Indexed by `(page, lang)`
 - Metadata table stores:
   - `last_pull_rfc3339`
@@ -175,6 +192,12 @@ go mod tidy
 go run .
 ```
 
+Run tests:
+
+```bash
+go test ./...
+```
+
 Build binary:
 
 ```bash

config.go

@@ -1,8 +1,10 @@
 package main
 
 import (
-	"log"
+	"errors"
+	"fmt"
 	"os"
+	"slices"
 	"time"
 )
 
@@ -18,44 +20,72 @@ type Config struct {
 	LogLevel                 string
 }
 
-func LoadConfig() Config {
-	return Config{
+var validLogLevels = []string{"debug", "info", "warn", "error"}
+
+// LoadConfig reads configuration from the environment.
+// It collects every problem instead of stopping at the first one,
+// so a misconfigured deployment reports all mistakes at once.
+func LoadConfig() (Config, error) {
+	var errs []error
+
+	requireEnv := func(k string) string {
+		v := os.Getenv(k)
+		if v == "" {
+			errs = append(errs, fmt.Errorf("missing required env: %s", k))
+		}
+		return v
+	}
+
+	envDuration := func(k string, def time.Duration) time.Duration {
+		v := os.Getenv(k)
+		if v == "" {
+			return def
+		}
+		d, err := time.ParseDuration(v)
+		if err != nil {
+			errs = append(errs, fmt.Errorf("invalid duration for %s: %q", k, v))
+			return def
+		}
+		return d
+	}
+
+	cfg := Config{
 		ListenAddr:               env("LISTEN_ADDR", ":8080"),
 		SQLitePath:               env("SQLITE_PATH", "/data/cacheppuccino.db"),
-		TranslationBaseURL:       mustEnv("TRANSLATION_BASE_URL"),
-		TranslationApplicationID: mustEnv("TRANSLATION_APPLICATION_ID"),
-		TranslationAPIKey:        mustEnv("TRANSLATION_API_KEY"),
+		TranslationBaseURL:       requireEnv("TRANSLATION_BASE_URL"),
+		TranslationApplicationID: requireEnv("TRANSLATION_APPLICATION_ID"),
+		TranslationAPIKey:        requireEnv("TRANSLATION_API_KEY"),
 		HTTPTimeout:              envDuration("HTTP_TIMEOUT", 30*time.Second),
 		PullInterval:             envDuration("PULL_INTERVAL", 10*time.Minute),
-		InitialPullDeadline:      envDuration("INITIAL_PULL_DEADLINE", 60*time.Second),
+		InitialPullDeadline:      envDuration("INITIAL_PULL_DEADLINE", 45*time.Second),
 		LogLevel:                 env("LOG_LEVEL", "info"),
 	}
-}
 
-func env(k, def string) string {
-	v := os.Getenv(k)
-	if v == "" {
-		return def
+	if cfg.HTTPTimeout <= 0 {
+		errs = append(errs, fmt.Errorf("HTTP_TIMEOUT must be positive, got %s", cfg.HTTPTimeout))
+	}
+	if cfg.PullInterval <= 0 {
+		errs = append(errs, fmt.Errorf("PULL_INTERVAL must be positive, got %s", cfg.PullInterval))
+	}
+	// Zero means "no deadline" for the initial pull; only negatives are invalid.
+	if cfg.InitialPullDeadline < 0 {
+		errs = append(errs, fmt.Errorf("INITIAL_PULL_DEADLINE must not be negative, got %s", cfg.InitialPullDeadline))
 	}
-	return v
-}
 
-func mustEnv(k string) string {
-	v := os.Getenv(k)
-	if v == "" {
-		log.Fatalf("missing required env: %s", k)
+	if !slices.Contains(validLogLevels, cfg.LogLevel) {
+		errs = append(errs, fmt.Errorf("invalid LOG_LEVEL: %q (valid: debug, info, warn, error)", cfg.LogLevel))
 	}
-	return v
+
+	if len(errs) > 0 {
+		return Config{}, errors.Join(errs...)
+	}
+	return cfg, nil
 }
 
-func envDuration(k string, def time.Duration) time.Duration {
+func env(k, def string) string {
 	v := os.Getenv(k)
 	if v == "" {
 		return def
 	}
-	d, err := time.ParseDuration(v)
-	if err != nil {
-		return def
-	}
-	return d
+	return v
 }