feat: add print service by jeastham1993 · Pull Request #139 · DataDog/stickerlandia

jeastham1993 · 2026-01-15T21:46:16Z

No description provided.

Copilot

Pull request overview

This PR adds a comprehensive print service implementation for Stickerlandia, introducing a complete print job management system with printer registration, job queuing, polling, and acknowledgment capabilities.

Changes:

Implements core print service functionality including printer registration, print job submission, polling, and acknowledgment
Adds comprehensive unit and integration test coverage for all major components
Introduces a Blazor-based printer client application for job processing

Reviewed changes

Copilot reviewed 242 out of 268 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
print-service/tests/Stickerlandia.PrintService.UnitTest/PrintJobTests/*.cs	Unit tests for print job query and command handlers
print-service/tests/Stickerlandia.PrintService.IntegrationTest/*.cs	Integration tests covering printer and print job workflows
print-service/tests/Stickerlandia.PrintService.Client.Tests/*.cs	Unit tests for the printer client application
print-service/src/Stickerlandia.PrintService.Core/*/.cs	Core domain models, handlers, and interfaces for print service
print-service/src/Stickerlandia.PrintService.Client/*/	Blazor-based printer client application
print-service/src/utils/Stickerlandia.PrintService.JwtGenerator/*.cs	JWT token generation utility
print-service/src/Stickerlandia.PrintService.Worker/*.cs	Background worker services for outbox processing

Files not reviewed (3)

print-service/infra/azure/.terraform.lock.hcl: Language not supported
print-service/infra/gcp/.terraform.lock.hcl: Language not supported
print-service/src/Stickerlandia.PrintService.Agnostic/Migrations/20260115122951_InitialCreate.Designer.cs: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-15T21:47:38Z

+    [JsonPropertyName("EventName")]
+    public string EventName { get; set; } = string.Empty;
+
+    [JsonPropertyName("printerName")]
+    public string PrinterName { get; set; } = string.Empty;
+
+    [JsonPropertyName("Key")]


The JsonPropertyName attributes use PascalCase ("EventName", "Key") which is inconsistent with the camelCase naming convention used in other response models in the same directory (e.g., "printerId", "printerName"). Consider changing to "eventName" and "key" for consistency.

Suggested change

[JsonPropertyName("EventName")]

public string EventName { get; set; } = string.Empty;

[JsonPropertyName("printerName")]

public string PrinterName { get; set; } = string.Empty;

[JsonPropertyName("Key")]

[JsonPropertyName("eventName")]

public string EventName { get; set; } = string.Empty;

[JsonPropertyName("printerName")]

public string PrinterName { get; set; } = string.Empty;

[JsonPropertyName("key")]

Copilot · 2026-01-15T21:47:38Z

+    [JsonPropertyName("printerId")]
+    public string PrinterId { get; set; } = "";
+
+    [JsonPropertyName("EventName")]


The JsonPropertyName attribute uses PascalCase ("EventName") which is inconsistent with the camelCase naming convention used in other DTO models (e.g., "printerId", "printerName"). Consider changing to "eventName" for consistency.

Suggested change

[JsonPropertyName("EventName")]

[JsonPropertyName("eventName")]

Copilot · 2026-01-15T21:47:39Z

+    [JsonPropertyName("printerId")]
+    public string PrinterId { get; set; }
+
+    [JsonPropertyName("EventName")]


The JsonPropertyName attribute uses PascalCase ("EventName") which is inconsistent with the camelCase naming convention used for other properties in this class ("printerId", "printerName"). Consider changing to "eventName" for consistency.

Suggested change

[JsonPropertyName("EventName")]

[JsonPropertyName("eventName")]

Copilot · 2026-01-15T21:47:39Z

+        Key = printer.Key;
+    }
+
+    [JsonPropertyName("Key")] public string Key { get; set; } = string.Empty;


The JsonPropertyName attribute uses PascalCase ("Key") which is inconsistent with the camelCase naming convention. Consider changing to "key" for consistency with standard JSON naming conventions.

Suggested change

[JsonPropertyName("Key")] public string Key { get; set; } = string.Empty;

[JsonPropertyName("key")] public string Key { get; set; } = string.Empty;

Copilot · 2026-01-15T21:47:39Z

+        PrinterId = printer?.Id?.Value ?? "";
+    }
+
+    [JsonPropertyName("EventName")]


The JsonPropertyName attribute uses PascalCase ("EventName") which is inconsistent with the camelCase naming convention used for other properties ("eventVersion", "printerId"). Consider changing to "eventName" for consistency.

Suggested change

[JsonPropertyName("EventName")]

[JsonPropertyName("eventName")]

Copilot · 2026-01-15T21:47:39Z

+        StickerId = printJob.StickerId;
+    }
+
+    [JsonPropertyName("EventName")]


The JsonPropertyName attribute uses PascalCase ("EventName") which is inconsistent with the camelCase naming convention used for other properties ("eventVersion", "printJobId", "printerId", "userId", "stickerId"). Consider changing to "eventName" for consistency.

Suggested change

[JsonPropertyName("EventName")]

[JsonPropertyName("eventName")]

Copilot · 2026-01-15T21:47:40Z

+        FailureReason = printJob.FailureReason ?? string.Empty;
+    }
+
+    [JsonPropertyName("EventName")]


The JsonPropertyName attribute uses PascalCase ("EventName") which is inconsistent with the camelCase naming convention used for other properties ("eventVersion", "printJobId", "printerId", "failureReason"). Consider changing to "eventName" for consistency.

Suggested change

[JsonPropertyName("EventName")]

[JsonPropertyName("eventName")]

Copilot · 2026-01-15T21:47:40Z

+        CompletedAt = printJob.CompletedAt ?? DateTimeOffset.UtcNow;
+    }
+
+    [JsonPropertyName("EventName")]


The JsonPropertyName attribute uses PascalCase ("EventName") which is inconsistent with the camelCase naming convention used for other properties ("eventVersion", "printJobId", "printerId", "completedAt"). Consider changing to "eventName" for consistency.

Suggested change

[JsonPropertyName("EventName")]

[JsonPropertyName("eventName")]

Copilot · 2026-01-15T21:47:40Z

+
+public abstract record DomainEvent
+{
+    [JsonPropertyName("EventName")]


The JsonPropertyName attribute uses PascalCase ("EventName") which is inconsistent with the camelCase naming convention used for "eventVersion". Consider changing to "eventName" for consistency.

Suggested change

[JsonPropertyName("EventName")]

[JsonPropertyName("eventName")]

scottgerring

Review: SHA tagging inconsistency identified

scottgerring

Memory limit

scottgerring

mise task

scottgerring

Code quality checks

scottgerring · 2026-01-20T08:18:45Z

+        }
+
+        services
+            .AddStickerlandiaUserManagement();


Copy-paste: calls AddStickerlandiaUserManagement() - needs renaming

scottgerring · 2026-01-20T08:18:59Z

+{
+  "$schema": "http://json.schemastore.org/launchsettings.json",
+  "profiles": {
+    "Stickerlandia.UserManagement.MigrationService": {


Copy-paste: profile name is Stickerlandia.UserManagement.MigrationService

…/ViewModels/RegisterPrinterResponse.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

jeastham1993 · 2026-01-15T21:53:49Z

@scottgerring could you review this file in particular please to check I've not broken anything propertly

jeastham1993 · 2026-01-15T21:54:35Z

@scottgerring could you have a look at these auth file changes in particular, just to see if you can spot anything obvious I've broken.

… into feat/print-servic:wq

Implement print station UI for the sticker printing microservice: - Add PrintStation page with printer list and status polling - Add PrintDialog for selecting stickers and submitting print jobs - Add RegisterPrinterDialog for admin printer registration with API key display - Add print button to StickerDetail page with navigation state - Fix Sidebar link to Print Station - Add print.js service with proper auth and error handling Security & stability improvements from code review: - Add AbortController for all async fetch operations - Add mounted ref checks to prevent state updates after unmount - Add double-submit protection with ref guards - Add userId validation before API calls - Add URL parameter encoding (encodeURIComponent) - Add token validation in getHeaders() to prevent "Bearer undefined" - Fix stale closure in escape handler with useCallback Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

print frontend into print-service

claude · 2026-02-10T08:59:44Z

Code Review

I've found 9 issues in this PR that need attention:

Critical Bugs 🐛

HttpClient disposed before use (AuthenticationExtensions.cs:85)
- HttpClientHandler and HttpClient are declared with using var, so they are disposed when the if block exits at line 102
- However, the IssuerSigningKeyResolver lambda captures httpClient and uses it later during token validation
- This will throw ObjectDisposedException on every token validation attempt when MetadataAddress is configured
- Fix: Remove the using keyword from both declarations
Wrong repository method called (AcknowledgePrintJobCommandHandler.cs:57)
- GetPrinterByKeyAsync is called with a printer ID (format: EVENTNAME-PRINTERNAME) instead of an API key
- This method searches by the Key field (base64-encoded GUID), so it always returns null
- Results in eventName always being "unknown" in metrics and printer.RecordJobProcessed() never executing
- Fix: Use a method that looks up by printer ID, not API key
OutboxProcessor missing event handlers (OutboxProcessor.cs:60)
- Switch statement only handles printers.registered.v1 events
- Print job events (printJobs.queued.v1, printJobs.completed.v1, printJobs.failed.v1) fall through to default case
- All print job events are marked as Failed with "Unknown event type"
- In agnostic (Postgres/Kafka) deployment, print job lifecycle events are never published
- Fix: Add cases for print job event types
Duplicate EventId compilation error (LogMessages.cs:96)
- Both StartingMessageProcessor (line 56) and StoppingMessageProcessor (line 96) use EventId = 5
- Source generator for [LoggerMessage] will produce error: SYSLIB1002
- Fix: Change line 96 to use EventId = 11
Route parameter ignored (RegisterPrinterEndpoint.cs:22)
- Route has /event/{eventName} parameter but uses request.EventName from body instead
- Caller can POST to /event/A with body {"eventName": "B"} and printer registers under "B"
- Violates REST semantics and could cause authorization issues
- Fix: Override request.EventName = eventName or validate they match

Security Issue 🔒

Hardcoded JWT signing key (AuthenticationExtensions.cs:127)
- Fallback key DRjd/GnduI3Efzen9V9BvbNUfc/VKgXltV7Kbk9sMkY= is publicly visible in source code
- Same key appears in appsettings.Development.json and TestConstants.cs
- Anyone can forge valid JWT tokens if config is missing or Authentication:Mode is unconfigured
- Fix: Throw exception if no signing key configured instead of using hardcoded fallback

Quality Issues

Wrong log message (Program.cs:216)
- Log states "UserManagement API started" but this is the Print Service
- Per CLAUDE.md, this is a Print Management Service
- Fix: Change to "Print Service API started"
RFC 7807 not implemented (GlobalExceptionHandler.cs:102)
- CLAUDE.md requires RFC 7807 Problem Details format
- Current response uses {Status: "Error", Message: "..."} instead of RFC 7807 fields
- Fix: Use ProblemDetails class or implement RFC 7807 structure (type, title, status, detail, instance)
Hardcoded log value (OutboxProcessor.cs:43)
- Log call passes literal 5 instead of outboxItems.Count
- Log message always reports 5 items regardless of actual count
- Fix: Change to LogMessages.LogUnprocessedOutboxItems(logger, outboxItems.Count, null)

Priority: Issues 1-6 are critical and should be fixed before merging. Issues 7-9 are quality improvements but less urgent.

jeastham1993 added 12 commits January 14, 2026 12:32

feat: initial commit of print service

9b07543

feat: add base print job functionality

a3d9c3c

feat: add printer polling API

5ce7057

feat: add printer endpoints

01f673e

feat: printer implementation

b598d13

feat: finish initial print service implementation

cd56551

Merge branch 'main' into feat/print-service

0f13fed

feat: implement print service

862cca0

feat: implement print service

e78033c

chore: add print service to docker compose

bf1ce11

Fix test issues with print service

df3ce00

chore: add print service workflow

87af931

jeastham1993 requested a review from Copilot January 15, 2026 21:46

Copilot AI reviewed Jan 15, 2026

View reviewed changes

github-advanced-security AI found potential problems Jan 15, 2026

View reviewed changes

Comment thread .github/workflows/print-service.yml Fixed

Comment thread .github/workflows/print-service.yml Fixed

jeastham1993 added 8 commits January 16, 2026 07:53

chore: add AWS tests back into workflow

99850bc

chore: update docs

1ab56b3

fix: update event naming

38f6ed7

chore: update Claude and README

6476dd0

chore: updating infra code

0deb4b5

docs: add initial code review

62ea483

Merge branch 'main' into feat/print-service

5ae982c

fix: add .NET 10 Lambda build image

7eb80e6

github-advanced-security AI found potential problems Jan 19, 2026

View reviewed changes

Comment thread .github/workflows/print-service.yml Fixed

Comment thread .github/workflows/print-service.yml Fixed

This comment was marked as outdated.

Sign in to view