Lab 03 Defects

[antonyboom]

Lab 03 (Spring Boot / Java Track) — Defect Report

Lab: lab-03-generation-and-refactoring-java.md
Track affected: 🟩 Spring Boot / Java only (.NET, Kotlin and Swift versions are separate files)
Date: 2026-09-21
Status: Open — lab completed with workarounds, lab document not yet corrected
Related: LAB_01_SPRINGBOOT_DEFECTS.md · LAB_02_SPRINGBOOT_DEFECTS.md

---

Summary

Lab 03 inherits the layout problems of Lab 02 and adds a more serious one: Part 2's worked
example refactors a class that does not exist, bearing no resemblance to the legacy code
actually shipped in the repository. Part 1 also asks participants to build endpoints that are
already built.

The lab was completed anyway — 238 tests green — but almost none of it could be followed
literally.

#	Severity	Defect	Participant impact
1	🔴 Blocking	Part 2's worked example refactors a non-existent class	Code is unwritable; example is unrelated to the real class
2	🔴 Blocking	Part 1 asks for CRUD endpoints that already exist	Most of a 20-minute section is a no-op
3	🔴 Blocking	TaskStatus values are wrong throughout	Every status sample throws at runtime
4	🟡 Moderate	Tests are generated after the refactor	Cannot prove behaviour was preserved
5	🟡 Moderate	Sample code calls APIs that do not exist	Nothing compiles
6	🟡 Moderate	Part 3's example re-introduces JPA into the domain	Contradicts the repo's architecture
7	🟡 Moderate	Part 4 is a destructive rename presented as a normal step	Would break DTOs, migrations and ~240 tests
8	🟡 Moderate	Wrong file paths again	Files land in the wrong module
9	🟢 Minor	mvn spring-boot:run fails as documented	App will not start
10	🟢 Minor	Stale sample dates	"Valid" curl examples return 400
11	🟢 Minor	Success criteria require PUT /api/tasks/{id}	Conflicts with the existing PATCH

---

Defect 1 — Part 2's worked example refactors a class that does not exist

Severity: 🔴 Blocking

Where

Part 2.3, "Expected Refactored Code", and Part 2.4's test checklist.

What

The lab presents a ~120-line RefactoredTaskProcessor operating on a batch of tasks:

public ProcessingResult processTaskBatch(List<TaskItem> tasks) { ... }
private void processSingleTask(TaskItem task, ProcessingResult result) { ... }
private boolean isTaskValid(TaskItem task) { ... }

It depends on TaskItem, ProcessingResult, TaskOutputWriter, ProcessingException,
TaskStatus.PROCESSING and TaskRepository.save(TaskItem).

None of these exist. TaskItem appears nowhere in src-springboot. TaskStatus has no
PROCESSING constant. TaskRepository saves domain Task, not TaskItem.

The class the repository actually ships is a string transformer:

public String processTask(int id, String data, int type, boolean flag)

It inverts letter case, replaces spaces with underscores, truncates to 50 characters, sleeps
100 ms, and writes a file — with 6 levels of nesting and two swallowed exceptions.

Why it matters

The worked example is not a refactoring of the legacy class. It is a different program. A
participant who follows Part 2.3 writes code that neither compiles nor relates to the file
they were told to open. Part 2.4's verification checklist then asks them to confirm
behaviours (TaskOutputWriter is invoked for successful tasks, Result contains correct success/failure counts) that the real class has no concept of.

Proposed fix

Replace the worked example with an actual refactoring of processTask. The genuinely useful
teaching points are all present in the real class and none require inventing types:

• 6 levels of nesting → guard clauses and a switch
• int type → a ProcessingType enum
• 50, ' ', 100 → named constants
• result += in a loop → StringBuilder
• file I/O inline → extracted behind a port
• two empty catch blocks → log and throw

Workaround applied

Ignored the example and refactored the real class. Introduced ProcessingType,
TaskOutputWriter (port), FileTaskOutputWriter (adapter) and TaskProcessingException.
Public signature unchanged, so lab-04's
commit exercise still references a valid path.

---

Defect 2 — Part 1 asks for CRUD endpoints that already exist

Severity: 🔴 Blocking

Where

Part 1 in its entirety (20 minutes of a 45-minute lab).

What

You have the POST /api/tasks endpoint from Lab 2. Now complete the REST API with GET, PUT,
and DELETE operations.

TaskController already exposes nine endpoints:

Method	Path
POST	/api/tasks
GET	/api/tasks
GET	/api/tasks/{id}
GET	/api/tasks/due-soon
PATCH	/api/tasks/{id}
PUT	/api/tasks/{id}/start
PUT	/api/tasks/{id}/complete
PUT	/api/tasks/{id}/cancel
DELETE	/api/tasks/{id}

Sections 1.3 (GET by ID) and 1.5 (DELETE) are entirely redundant. 1.4 (PUT update) duplicates
the existing PATCH.

Why it matters

Same failure mode as Lab 02 Defect 3: the lab's narrative assumes a greenfield state that the
repository left behind two labs ago. Participants either rewrite working endpoints or sit idle.

The one genuine gap is buried in 1.2: GET /api/tasks supports no status filter.

Proposed fix

Rewrite Part 1 around what is actually missing. The status filter is a good exercise on its
own — it touches the service, the controller, error handling for an invalid enum value, and
interacts with the existing priority filter.

Workaround applied

Implemented only the gap: GET /api/tasks?status=IN_PROGRESS, combinable with ?priority=.
Added TaskService.findTasks(Collection<Priority>, TaskStatus) plus 5 endpoint tests.

---

Defect 3 — TaskStatus values are wrong throughout

Severity: 🔴 Blocking

Where

Part 1.2 (service prompt, controller sample, and the explanatory note).

What

Note: The domain model uses TaskStatus enum (TODO/IN_PROGRESS/DONE) rather than a
boolean completed field.

The actual enum is:

public enum TaskStatus { PENDING, IN_PROGRESS, COMPLETED, CANCELLED }

The lab's sample controller hard-codes the wrong set in its own error message:

"Invalid status: " + status + ". Valid values: TODO, IN_PROGRESS, DONE"

Why it matters

TaskStatus.valueOf("TODO") throws. TaskStatus.valueOf("DONE") throws. The lab is half
right — it correctly warns that the model is not a boolean — then supplies three values of
which only one is real, and omits CANCELLED entirely.

Proposed fix

Correct to PENDING, IN_PROGRESS, COMPLETED, CANCELLED in all three places.

Workaround applied

Used the real values. The regression test asserts the 400 message names COMPLETED when
DONE is supplied, which pins this defect so it cannot silently return.

---

Defect 4 — Tests are generated after the refactor

Severity: 🟡 Moderate

Where

Part 2 ordering: 2.3 "Refactor with /refactor Command" → 2.4 "Generate Tests for Refactored Code".

What

The lab refactors first, then runs /tests against the result.

Why it matters

This is the wrong order for legacy code, and it undermines the lab's own claim that
refactoring "preserves behaviour". Tests written against already-refactored code assert what
the new code does. If the refactor silently changed behaviour, those tests enshrine the bug.

The legacy class has at least one behaviour nobody would guess: processTask(1, " ", 2, false)
returns "", because " ".split(" ") yields an empty array. The obvious refactor —
words[0] followed by a loop — throws ArrayIndexOutOfBoundsException. A test written after
that refactor would never catch it.

Proposed fix

Insert a step before 2.3: write characterization tests against the unmodified class.
Explain that they describe what the code does, not what it should do, and must pass both
before and after. This is a genuinely valuable technique and the lab is one step away from
teaching it.

Workaround applied

Wrote 17 characterization tests first, covering guards, all four branches, the 50-character
truncation boundary, and the " " split edge case. All passed before the refactor and still
pass after.

---

Defect 5 — Sample code calls APIs that do not exist

Severity: 🟡 Moderate

Where

Parts 1.2, 1.3, 1.4, 1.5 — every "Expected Output" block.

What

Lab sample	Reality
task.getId() returning UUID	returns TaskId (a record)
task.isCompleted() into a TaskResponse field	TaskResponse carries status, not a boolean
TaskService.findTaskById(UUID)	findTask(TaskId)
TaskService.updateTask(UUID, UpdateTaskRequest)	four focused methods: updateTaskTitle/Description/Priority/DueDate
TaskService.deleteTask(UUID)	deleteTask(TaskId)
taskRepository.findById(id) with a UUID	port takes TaskId
EntityNotFoundException	TaskService.TaskNotFoundException
new TaskResponse(...) positional construction	TaskResponse.from(Task) factory

Why it matters

Every generated block needs rewriting before it compiles. The TaskResponse example is the
worst: it constructs the record positionally with eight arguments in an order that does not
match the real record, so it would compile only after the participant reorders and retypes
every field.

Proposed fix

Regenerate all samples against the real API surface, and prefer TaskResponse.from(task)
over positional construction — it already exists precisely to avoid this.

---

Defect 6 — Part 3's example re-introduces JPA into the domain

Severity: 🟡 Moderate

Where

Part 3.2, "Apply: Wrap Primitives".

What

The "After" sample shows:

@Entity
public class TaskItem {
    @EmbeddedId
    private TaskId id;
    @Enumerated(EnumType.STRING)
    private TaskStatus status;
}

@Embeddable
public class TaskId { private UUID value; }

Why it matters

Identical to Lab 02 Defect 2. .github/instructions/springboot.instructions.md requires
"Domain → no deps (pure Java, no Spring)". The repository keeps the domain pure and maps
via TaskEntity + TaskMapper. Making TaskId @Embeddable drags JPA into the domain module.

There is a second irony: the lab presents this as an improvement, but the repository already
satisfies the underlying rule — TaskId is a record wrapping UUID, and TaskStatus and
Priority are enums, not ints. The domain already passes this Object Calisthenics rule.

Proposed fix

Replace the example with one drawn from code that actually has primitive obsession. Good
real candidates: LegacyTaskProcessor's int type and boolean flag parameters — which is
exactly what Part 2 fixes, so the two parts could reinforce each other.

---

Defect 7 — Part 4 is a destructive rename presented as a normal step

Severity: 🟡 Moderate

Where

Part 4, "Multi-File Refactoring with Copilot Edits".

What

Rename the "title" field to "name" across all files in the working set.

Why it matters

As a Copilot Edits demo the mechanics are fine. As an instruction applied to this repository
it is destructive and no warning is given. title appears in the domain aggregate, the JPA
entity, the mapper, CreateTaskRequest, UpdateTaskRequest, TaskResponse, the controller,
both Flyway migrations, and roughly 240 tests. It also breaks the published API contract
for no functional gain.

The listed working set is incomplete anyway — it omits TaskEntity, TaskMapper, the
migrations and four of the five test classes — so following it produces a partial rename
that leaves the codebase broken.

Proposed fix

Mark Part 4 explicitly as a throwaway demonstration: do it on a scratch branch, review the
diff, then discard. Or pick a genuinely safe multi-file change.

Workaround applied

Skipped deliberately.

---

Defect 8 — Wrong file paths again

Severity: 🟡 Moderate

Where

Part 1.2 Step 2 and Part 3.

What

#file:src-springboot/presentation/controllers/TaskController.java

There is no presentation directory. The real path is
src-springboot/taskmanager-api/src/main/java/com/example/taskmanager/api/controllers/TaskController.java.

Part 1.1 gets this right, which makes the inconsistency more confusing, not less.

Proposed fix

Same as Lab 02 Defect 1: correct all paths, rename "presentation" to "api" throughout.

---

Defect 9 — mvn spring-boot:run fails as documented

Severity: 🟢 Minor

Where

Part 1.6, "Run and Test".

What

Identical to Lab 02 Defect 5. The bare command fails twice over: the default profile targets
PostgreSQL, and -pl taskmanager-api without a prior install resolves stale jars.

Proposed fix

cd src-springboot
mvn clean install -DskipTests
mvn spring-boot:run -pl taskmanager-api -Dspring-boot.run.profiles=dev

Windows PowerShell requires quoting the -D argument.

---

Defect 10 — Stale sample dates

Severity: 🟢 Minor

Where

Part 1.6 curl examples: 2026-04-30T12:00:00 and 2026-05-01T12:00:00.

What

Both are in the past as of 2026-09-21, so the create and update calls return 400 —
due dates must be in the future.

Proposed fix

Same as Lab 02 Defect 7: use relative wording or a far-future constant.

---

Defect 11 — Success criteria require PUT /api/tasks/{id}

Severity: 🟢 Minor

Where

Success Criteria: "Full CRUD API endpoints implemented (POST, GET, GET by ID, PUT, DELETE)".

What

The repository updates through PATCH /api/tasks/{id}, which is the correct verb for the
partial-update semantics actually implemented (every field optional). Adding PUT would
create two overlapping update endpoints.

Proposed fix

Accept PATCH in the success criteria, or explain why PUT is wanted alongside it.

---

Verification status of the reference implementation

cd src-springboot && mvn clean test → BUILD SUCCESS

Module	Tests	Before Lab 3	After
taskmanager-domain	65	65	—
taskmanager-application	106	84	+22
taskmanager-infrastructure	48	30	+18
taskmanager-api	54	36	+18
Total	273	215	+58

What was actually built

Lab section	Outcome
Part 1	?status= filter added; rest already existed
Part 2	LegacyTaskProcessor refactored, behaviour pinned by 17 characterization tests
Part 3	Verified — domain already complies; applied to the refactored processor
Part 4	Skipped (Defect 7)
Extension 1 — pagination	Done, in the application layer
Extension 2 — sorting	Done, title/priority/dueDate/createdAt, both directions
Extension 3 — response mapper	Skipped by decision — see below

Extension 3 skipped deliberately

The lab asks for a dedicated TaskResponseMapper component. TaskResponse.from(Task) already
exists, is used by every endpoint, and lives on the DTO that owns the mapping. Extracting a
@Component that delegates to it would add a class and an injected dependency while either
duplicating the factory or leaving it redundant.

Worth keeping in the lab as a discussion prompt — "is this extraction warranted here?" is
a better question than an instruction, and the honest answer for this codebase is no.

Pagination deviates from the lab's sample

The lab shows Page<TaskResponse> built from a Spring Data Pageable. That would require
Pageable on the TaskRepository domain port, pulling Spring Data into the domain module —
the violation already recorded as Lab 02 Defect 2.

Instead, filtering, sorting and slicing happen in the application and API layers, returning a
plain PagedResponse<T> record. Two consequences, both deliberate:

• GET /api/tasks changed shape from a bare array to {content, page, size, totalElements, totalPages, first, last}. A breaking change; 11 existing assertions were updated.
• Rows are loaded then sliced in memory. Fine at workshop scale, wrong at real scale.
  Recorded as a known gap rather than hidden.

Refactor outcome against the lab's 11 stated goals

All met: guard clauses replace 6-level nesting; ProcessingType replaces int type;
50/' '/100 became named constants; StringBuilder replaces += in a loop; file I/O
moved behind TaskOutputWriter; SLF4J added; both swallowed exceptions now log and throw;
longest method is 12 lines at ≤2 indentation levels.

Goal 11 (CompletableFuture) was not applied — it would change the public signature that
lab-04 later references in its commit
exercise. The lab marks it optional.

Deliberate behaviour change

The swallowed IOException and InterruptedException now surface as TaskProcessingException,
with the interrupt flag restored. This is the lab's goal #2 and is intentional; it is the only
observable change.

Bug found during implementation

Injecting a Path via @Value fails inside a web application context — Spring resolves it as
a ServletContext resource:

Failed to convert value of type 'java.lang.String' to required type 'java.nio.file.Path';
Could not retrieve file for ServletContext resource [/]

FileTaskOutputWriter now accepts a String and converts internally. Worth a troubleshooting
entry if the lab ever recommends externalising a path.

---

Suggested triage

Before the next workshop run — Defects 1–3. Part 2's worked example is unusable, Part 1 is
largely a no-op, and every status sample throws.

Soon after — Defects 4–8. Defect 4 in particular is a missed teaching opportunity rather
than just an error: characterization-first is the technique this lab should be demonstrating.

When convenient — Defects 9–11.