From aab73fa6d8c61825d437bbe3bc1501804cbdb8ad Mon Sep 17 00:00:00 2001 From: Pavlo Sushkov Date: Wed, 12 Aug 2026 18:03:21 +0300 Subject: [PATCH] feat: add candidate ownership to profiles --- .../config/CandidateProfileProperties.java | 7 + .../jobpilot/candidate/domain/Candidate.java | 53 ++++++ .../candidate/domain/CandidateProfile.java | 14 +- .../CandidateProfileRepository.java | 10 + .../repository/CandidateRepository.java | 9 + .../CandidateProfileBootstrapService.java | 39 +++- src/main/resources/candidate-profile.yml | 3 + .../db/migration/V14__candidate_ownership.sql | 39 ++++ .../candidate/CandidateProfileTestData.java | 16 +- .../V14CandidateOwnershipMigrationTest.java | 180 ++++++++++++++++++ ...didateProfileBootstrapIntegrationTest.java | 59 ++++++ .../CandidateProfileBootstrapServiceTest.java | 76 ++++++-- .../jobpilot/jobs/PostgresPersistenceIT.java | 7 +- 13 files changed, 486 insertions(+), 26 deletions(-) create mode 100644 src/main/java/com/jobpilot/candidate/domain/Candidate.java create mode 100644 src/main/java/com/jobpilot/candidate/repository/CandidateRepository.java create mode 100644 src/main/resources/db/migration/V14__candidate_ownership.sql create mode 100644 src/test/java/com/jobpilot/candidate/V14CandidateOwnershipMigrationTest.java diff --git a/src/main/java/com/jobpilot/candidate/config/CandidateProfileProperties.java b/src/main/java/com/jobpilot/candidate/config/CandidateProfileProperties.java index 8a18f7d..d6d6e66 100644 --- a/src/main/java/com/jobpilot/candidate/config/CandidateProfileProperties.java +++ b/src/main/java/com/jobpilot/candidate/config/CandidateProfileProperties.java @@ -1,5 +1,6 @@ package com.jobpilot.candidate.config; +import com.fasterxml.jackson.annotation.JsonIgnore; import com.jobpilot.candidate.domain.CandidateLanguageLevel; import com.jobpilot.candidate.domain.CandidateSkillCategory; import com.jobpilot.candidate.domain.ProjectType; @@ -23,6 +24,12 @@ @Validated @ConfigurationProperties("jobpilot.candidate-profile") public record CandidateProfileProperties( + // Stable key of the persistent Candidate that owns this profile. Ownership is not a + // candidate fact, so @JsonIgnore keeps it out of the source-hash fingerprint below: + // re-keying the owner must not read as "facts changed without increasing profileVersion", + // and the fingerprint of an already stored version must stay exactly what it was. + @JsonIgnore + @NotBlank @Size(max = 100) @Pattern(regexp = "[a-z0-9]+(?:-[a-z0-9]+)*") String candidateKey, @Positive int profileVersion, @NotBlank @Size(max = 200) String fullName, @NotBlank @Size(max = 300) String location, diff --git a/src/main/java/com/jobpilot/candidate/domain/Candidate.java b/src/main/java/com/jobpilot/candidate/domain/Candidate.java new file mode 100644 index 0000000..2d9a2b0 --- /dev/null +++ b/src/main/java/com/jobpilot/candidate/domain/Candidate.java @@ -0,0 +1,53 @@ +package com.jobpilot.candidate.domain; + +import jakarta.persistence.Column; +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.GenerationType; +import jakarta.persistence.Id; +import jakarta.persistence.Table; +import java.time.Instant; + +/** + * The owner of one or more {@link CandidateProfile} versions. + * + *

Deliberately minimal: it establishes ownership and nothing else. Identity, credentials, + * contact details and preferences belong to a later phase, and none of them are needed to say + * which candidate a stored profile version describes. + * + *

{@code stableKey} is the only identity that survives a database rebuild, so configuration + * and the seeded row are matched on it rather than on the generated id. + * + *

Ownership is modelled one-directionally from the profile. Nothing today needs to walk from a + * candidate to its profile versions, and a mapped collection would only invite loading them all. + */ +@Entity +@Table(name = "candidates") +public class Candidate { + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + private Long id; + @Column(nullable = false, unique = true, length = 100) + private String stableKey; + @Column(nullable = false) + private Instant createdAt; + @Column(nullable = false) + private Instant updatedAt; + + protected Candidate() { + } + + public Candidate(String stableKey, Instant now) { + if (stableKey == null || stableKey.isBlank()) { + throw new IllegalArgumentException("Candidate stable key is required"); + } + this.stableKey = stableKey.trim(); + this.createdAt = now; + this.updatedAt = now; + } + + public Long getId() { return id; } + public String getStableKey() { return stableKey; } + public Instant getCreatedAt() { return createdAt; } + public Instant getUpdatedAt() { return updatedAt; } +} diff --git a/src/main/java/com/jobpilot/candidate/domain/CandidateProfile.java b/src/main/java/com/jobpilot/candidate/domain/CandidateProfile.java index 9870caf..e1d270a 100644 --- a/src/main/java/com/jobpilot/candidate/domain/CandidateProfile.java +++ b/src/main/java/com/jobpilot/candidate/domain/CandidateProfile.java @@ -7,6 +7,8 @@ import jakarta.persistence.GeneratedValue; import jakarta.persistence.GenerationType; import jakarta.persistence.Id; +import jakarta.persistence.JoinColumn; +import jakarta.persistence.ManyToOne; import jakarta.persistence.OneToMany; import jakarta.persistence.OrderBy; import jakarta.persistence.Table; @@ -15,6 +17,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Objects; @Entity @Table(name = "candidate_profiles") @@ -22,6 +25,13 @@ public class CandidateProfile { @Id @GeneratedValue(strategy = GenerationType.IDENTITY) private Long id; + @ManyToOne(fetch = FetchType.LAZY, optional = false) + @JoinColumn(name = "candidate_id", nullable = false) + private Candidate candidate; + /** + * Still globally unique in the schema, not unique per candidate. Making version and active + * uniqueness candidate-scoped is a separate migration. + */ @Column(nullable = false, unique = true) private int profileVersion; @Column(nullable = false, length = 200) @@ -67,11 +77,12 @@ public class CandidateProfile { protected CandidateProfile() { } - public CandidateProfile(int profileVersion, String fullName, String location, + public CandidateProfile(Candidate candidate, int profileVersion, String fullName, String location, String educationInstitution, String degree, int studyStartYear, Integer studyEndYear, boolean currentStudent, boolean finalYearStudent, BigDecimal commercialJavaExperienceYears, String sourceHash, Instant now, boolean active) { + this.candidate = Objects.requireNonNull(candidate, "Owning candidate is required"); this.profileVersion = profileVersion; this.fullName = fullName; this.location = location; @@ -118,6 +129,7 @@ public void deactivate(Instant now) { } public Long getId() { return id; } + public Candidate getCandidate() { return candidate; } public int getProfileVersion() { return profileVersion; } public String getFullName() { return fullName; } public String getLocation() { return location; } diff --git a/src/main/java/com/jobpilot/candidate/repository/CandidateProfileRepository.java b/src/main/java/com/jobpilot/candidate/repository/CandidateProfileRepository.java index 425a98d..92d9e8f 100644 --- a/src/main/java/com/jobpilot/candidate/repository/CandidateProfileRepository.java +++ b/src/main/java/com/jobpilot/candidate/repository/CandidateProfileRepository.java @@ -5,6 +5,16 @@ import org.springframework.data.jpa.repository.JpaRepository; public interface CandidateProfileRepository extends JpaRepository { + Optional findByCandidateIdAndActiveTrue(Long candidateId); + + Optional findByCandidateIdAndProfileVersion(Long candidateId, int profileVersion); + + /** + * Installation-wide lookups, meaning "the one active profile in the whole system". Still + * correct only because a single candidate exists; {@code JobAnalysisService} and + * {@code ResumeGenerationService} read the active profile this way and are migrated to + * candidate-scoped access in a later phase. + */ Optional findByActiveTrue(); Optional findByProfileVersion(int profileVersion); diff --git a/src/main/java/com/jobpilot/candidate/repository/CandidateRepository.java b/src/main/java/com/jobpilot/candidate/repository/CandidateRepository.java new file mode 100644 index 0000000..2656b5f --- /dev/null +++ b/src/main/java/com/jobpilot/candidate/repository/CandidateRepository.java @@ -0,0 +1,9 @@ +package com.jobpilot.candidate.repository; + +import com.jobpilot.candidate.domain.Candidate; +import java.util.Optional; +import org.springframework.data.jpa.repository.JpaRepository; + +public interface CandidateRepository extends JpaRepository { + Optional findByStableKey(String stableKey); +} diff --git a/src/main/java/com/jobpilot/candidate/service/CandidateProfileBootstrapService.java b/src/main/java/com/jobpilot/candidate/service/CandidateProfileBootstrapService.java index 3a31b5b..88621d1 100644 --- a/src/main/java/com/jobpilot/candidate/service/CandidateProfileBootstrapService.java +++ b/src/main/java/com/jobpilot/candidate/service/CandidateProfileBootstrapService.java @@ -3,9 +3,11 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.jobpilot.candidate.config.CandidateProfileProperties; +import com.jobpilot.candidate.domain.Candidate; import com.jobpilot.candidate.domain.CandidateProfile; import com.jobpilot.candidate.domain.CandidateProject; import com.jobpilot.candidate.repository.CandidateProfileRepository; +import com.jobpilot.candidate.repository.CandidateRepository; import com.jobpilot.common.Hashing; import java.time.Clock; import java.time.Instant; @@ -19,14 +21,17 @@ public class CandidateProfileBootstrapService { private static final Logger log = LoggerFactory.getLogger(CandidateProfileBootstrapService.class); private final CandidateProfileRepository profiles; + private final CandidateRepository candidates; private final CandidateProfileDefinitionValidator validator; private final ObjectMapper objectMapper; private final Clock clock; public CandidateProfileBootstrapService(CandidateProfileRepository profiles, + CandidateRepository candidates, CandidateProfileDefinitionValidator validator, ObjectMapper objectMapper, Clock clock) { this.profiles = profiles; + this.candidates = candidates; this.validator = validator; this.objectMapper = objectMapper; this.clock = clock; @@ -36,7 +41,8 @@ public CandidateProfileBootstrapService(CandidateProfileRepository profiles, public CandidateProfileBootstrapResult bootstrap(CandidateProfileProperties definition) { validator.validate(definition); String sourceHash = sourceHash(definition); - var active = profiles.findByActiveTrue(); + Candidate candidate = resolveCandidate(definition.candidateKey()); + var active = profiles.findByCandidateIdAndActiveTrue(candidate.getId()); if (active.isPresent()) { CandidateProfile current = active.get(); if (definition.profileVersion() < current.getProfileVersion()) { @@ -56,26 +62,39 @@ public CandidateProfileBootstrapResult bootstrap(CandidateProfileProperties defi } } - profiles.findByProfileVersion(definition.profileVersion()).ifPresent(existing -> { - throw new CandidateProfileVersionConflictException( - "Configured profileVersion already exists and cannot be overwritten"); - }); + profiles.findByCandidateIdAndProfileVersion(candidate.getId(), definition.profileVersion()) + .ifPresent(existing -> { + throw new CandidateProfileVersionConflictException( + "Configured profileVersion already exists and cannot be overwritten"); + }); Instant now = clock.instant(); active.ifPresent(current -> { current.deactivate(now); profiles.saveAndFlush(current); }); - CandidateProfile created = map(definition, sourceHash, now); + CandidateProfile created = map(candidate, definition, sourceHash, now); profiles.saveAndFlush(created); - log.info("Candidate profile bootstrap created: version={}, skills={}, languages={}, projects={}", - created.getProfileVersion(), created.getSkills().size(), + log.info("Candidate profile bootstrap created: candidate={}, version={}, skills={}, languages={}, projects={}", + candidate.getStableKey(), created.getProfileVersion(), created.getSkills().size(), created.getLanguages().size(), created.getProjects().size()); return new CandidateProfileBootstrapResult(created.getId(), created.getProfileVersion(), true); } - private CandidateProfile map(CandidateProfileProperties definition, String sourceHash, Instant now) { - CandidateProfile profile = new CandidateProfile( + /** + * V14 seeds the configured key, so both an upgraded and a freshly migrated database already + * hold this row and the lookup is what normally runs. Creating a missing one keeps a re-keyed + * configuration deterministic instead of failing at startup; the unique stable key makes + * repeated bootstrap resolve the same row rather than adding another owner. + */ + private Candidate resolveCandidate(String stableKey) { + return candidates.findByStableKey(stableKey) + .orElseGet(() -> candidates.saveAndFlush(new Candidate(stableKey, clock.instant()))); + } + + private CandidateProfile map(Candidate candidate, CandidateProfileProperties definition, + String sourceHash, Instant now) { + CandidateProfile profile = new CandidateProfile(candidate, definition.profileVersion(), definition.fullName().trim(), definition.location().trim(), definition.educationInstitution().trim(), definition.degree().trim(), definition.studyStartYear(), definition.studyEndYear(), definition.currentStudent(), diff --git a/src/main/resources/candidate-profile.yml b/src/main/resources/candidate-profile.yml index d8145f4..fbc51b1 100644 --- a/src/main/resources/candidate-profile.yml +++ b/src/main/resources/candidate-profile.yml @@ -1,5 +1,8 @@ jobpilot: candidate-profile: + # Stable key of the persistent candidate owning this profile; seeded by migration V14. + # Ownership only: it is not a candidate fact and does not affect the profile source hash. + candidate-key: default profile-version: 1 full-name: Pavlo Sushkov location: Bucharest, Romania diff --git a/src/main/resources/db/migration/V14__candidate_ownership.sql b/src/main/resources/db/migration/V14__candidate_ownership.sql new file mode 100644 index 0000000..1e05894 --- /dev/null +++ b/src/main/resources/db/migration/V14__candidate_ownership.sql @@ -0,0 +1,39 @@ +-- Multi-user foundation, step 1: candidate_profiles gains an explicit owner. +-- +-- This migration establishes ownership ONLY. profile_version and active_slot are still +-- globally UNIQUE exactly as V2 declared them, so the schema continues to admit one active +-- profile and one profile per version for the whole installation rather than per candidate. +-- Those two constraints are the remaining single-candidate assumption and are rewritten in a +-- dedicated follow-up; nothing added here depends on them. + +CREATE TABLE candidates ( + id BIGINT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, + stable_key VARCHAR(100) NOT NULL UNIQUE CHECK (TRIM(stable_key) <> ''), + created_at TIMESTAMP WITH TIME ZONE NOT NULL, + updated_at TIMESTAMP WITH TIME ZONE NOT NULL +); + +-- The owner of every profile that exists today. 'default' is the value committed as +-- jobpilot.candidate-profile.candidate-key. Bootstrap resolves this row by its stable key and +-- never by id, so the seeded identity holds whatever id the sequence hands out here. +INSERT INTO candidates (stable_key, created_at, updated_at) +VALUES ('default', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP); + +ALTER TABLE candidate_profiles ADD COLUMN candidate_id BIGINT; + +-- Backfills every stored profile version, active and superseded alike: history belongs to the +-- same candidate as the current version. On a fresh database this updates zero rows, because +-- bootstrap only runs once the application starts, and the NOT NULL below still holds. +UPDATE candidate_profiles +SET candidate_id = (SELECT id FROM candidates WHERE stable_key = 'default') +WHERE candidate_id IS NULL; + +ALTER TABLE candidate_profiles ALTER COLUMN candidate_id SET NOT NULL; + +-- RESTRICT, not CASCADE: profile versions are referenced by resume_versions under the same +-- rule, and no code path deletes a candidate. +ALTER TABLE candidate_profiles ADD CONSTRAINT candidate_profiles_candidate_fk + FOREIGN KEY (candidate_id) REFERENCES candidates(id) ON DELETE RESTRICT; + +-- Every candidate-scoped profile lookup filters on this column. +CREATE INDEX candidate_profiles_candidate_idx ON candidate_profiles(candidate_id); diff --git a/src/test/java/com/jobpilot/candidate/CandidateProfileTestData.java b/src/test/java/com/jobpilot/candidate/CandidateProfileTestData.java index 0279a25..3336c9f 100644 --- a/src/test/java/com/jobpilot/candidate/CandidateProfileTestData.java +++ b/src/test/java/com/jobpilot/candidate/CandidateProfileTestData.java @@ -11,8 +11,11 @@ public final class CandidateProfileTestData { private CandidateProfileTestData() { } + public static final String CANDIDATE_KEY = "default"; + public static CandidateProfileProperties validProfile(int version) { return new CandidateProfileProperties( + CANDIDATE_KEY, version, "Test Candidate", "Bucharest, Romania", @@ -60,6 +63,17 @@ public static CandidateProfileProperties withFullName(CandidateProfileProperties source.projects()); } + /** Changes only the owning candidate, leaving every candidate fact untouched. */ + public static CandidateProfileProperties withCandidateKey(CandidateProfileProperties source, + String candidateKey) { + return new CandidateProfileProperties(candidateKey, source.profileVersion(), + source.fullName(), source.location(), source.educationInstitution(), + source.degree(), source.studyStartYear(), source.studyEndYear(), + source.currentStudent(), source.finalYearStudent(), + source.commercialJavaExperienceYears(), source.skills(), source.languages(), + source.projects()); + } + public static CandidateProfileProperties withVersion(CandidateProfileProperties source, int version) { return copy(source, version, source.fullName(), source.commercialJavaExperienceYears(), @@ -98,7 +112,7 @@ private static CandidateProfileProperties copy( BigDecimal experience, List skills, List languages, List projects) { - return new CandidateProfileProperties(version, fullName, source.location(), + return new CandidateProfileProperties(source.candidateKey(), version, fullName, source.location(), source.educationInstitution(), source.degree(), source.studyStartYear(), source.studyEndYear(), source.currentStudent(), source.finalYearStudent(), experience, skills, languages, projects); diff --git a/src/test/java/com/jobpilot/candidate/V14CandidateOwnershipMigrationTest.java b/src/test/java/com/jobpilot/candidate/V14CandidateOwnershipMigrationTest.java new file mode 100644 index 0000000..3388e5b --- /dev/null +++ b/src/test/java/com/jobpilot/candidate/V14CandidateOwnershipMigrationTest.java @@ -0,0 +1,180 @@ +package com.jobpilot.candidate; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.SQLException; +import java.sql.Statement; +import org.flywaydb.core.Flyway; +import org.junit.jupiter.api.Test; + +/** + * V14 on H2 in PostgreSQL mode, like the V8, V9, V12 and V13 migration tests. No container, no + * network, no live database. + * + *

Both paths matter: an existing installation already holds candidate_profiles rows that must + * come out owned, and a fresh database migrates through V14 with the table still empty. + */ +class V14CandidateOwnershipMigrationTest { + @Test + void backfillsExistingProfilesToTheSeededDefaultCandidate() throws Exception { + String url = "jdbc:h2:mem:v14backfill;MODE=PostgreSQL;DB_CLOSE_DELAY=-1;DATABASE_TO_LOWER=TRUE"; + Flyway.configure().dataSource(url, "sa", "").target("13").load().migrate(); + try (Connection connection = DriverManager.getConnection(url, "sa", ""); + Statement statement = connection.createStatement()) { + statement.executeUpdate(profile(1, false)); + statement.executeUpdate(profile(2, true)); + } + + Flyway.configure().dataSource(url, "sa", "").load().migrate(); + try (Connection connection = DriverManager.getConnection(url, "sa", ""); + Statement statement = connection.createStatement()) { + try (var rows = statement.executeQuery( + "SELECT id, stable_key FROM candidates")) { + assertThat(rows.next()).isTrue(); + assertThat(rows.getString("stable_key")).isEqualTo("default"); + assertThat(rows.next()).as("exactly one seeded candidate").isFalse(); + } + // Superseded versions belong to the same candidate as the active one: profile + // history is not reassigned or orphaned by gaining an owner. + try (var rows = statement.executeQuery(""" + SELECT p.profile_version, p.active, c.stable_key + FROM candidate_profiles p JOIN candidates c ON c.id = p.candidate_id + ORDER BY p.profile_version + """)) { + assertThat(rows.next()).isTrue(); + assertThat(rows.getInt("profile_version")).isEqualTo(1); + assertThat(rows.getBoolean("active")).isFalse(); + assertThat(rows.getString("stable_key")).isEqualTo("default"); + assertThat(rows.next()).isTrue(); + assertThat(rows.getInt("profile_version")).isEqualTo(2); + assertThat(rows.getBoolean("active")).isTrue(); + assertThat(rows.getString("stable_key")).isEqualTo("default"); + assertThat(rows.next()).isFalse(); + } + } + } + + @Test + void appliesToAFreshDatabaseWhereNoProfileExistsYet() throws Exception { + String url = migrate("v14fresh"); + try (Connection connection = DriverManager.getConnection(url, "sa", ""); + Statement statement = connection.createStatement()) { + try (var rows = statement.executeQuery( + "SELECT COUNT(*) c FROM flyway_schema_history " + + "WHERE version = '14' AND success = TRUE")) { + assertThat(rows.next()).isTrue(); + assertThat(rows.getInt("c")).isEqualTo(1); + } + // Bootstrap runs at application start, long after Flyway, so the NOT NULL below is + // reached with an empty table on every fresh install. + try (var rows = statement.executeQuery("SELECT COUNT(*) c FROM candidate_profiles")) { + assertThat(rows.next()).isTrue(); + assertThat(rows.getInt("c")).isZero(); + } + try (var rows = statement.executeQuery( + "SELECT COUNT(*) c FROM candidates WHERE stable_key = 'default'")) { + assertThat(rows.next()).isTrue(); + assertThat(rows.getInt("c")).isEqualTo(1); + } + } + } + + @Test + void requiresAnExistingOwnerForEveryProfile() throws Exception { + String url = migrate("v14ownership"); + try (Connection connection = DriverManager.getConnection(url, "sa", ""); + Statement statement = connection.createStatement()) { + assertThatThrownBy(() -> statement.executeUpdate(profile(1, true))) + .as("no candidate_id").isInstanceOf(SQLException.class); + assertThatThrownBy(() -> statement.executeUpdate(ownedProfile(1, true, 987_654L))) + .as("unknown candidate_id").isInstanceOf(SQLException.class); + + statement.executeUpdate(ownedProfile(1, true, defaultCandidateId(statement))); + } + } + + @Test + void keepsCandidateStableKeysUniqueAndNonBlank() throws Exception { + String url = migrate("v14candidatekey"); + try (Connection connection = DriverManager.getConnection(url, "sa", ""); + Statement statement = connection.createStatement()) { + assertThatThrownBy(() -> statement.executeUpdate(candidate("default"))) + .as("duplicate stable key").isInstanceOf(SQLException.class); + assertThatThrownBy(() -> statement.executeUpdate(candidate(" "))) + .as("blank stable key").isInstanceOf(SQLException.class); + + statement.executeUpdate(candidate("second-candidate")); + } + } + + /** + * The limitation this PR deliberately leaves in place: ownership exists, but a second + * candidate still cannot hold an active profile or reuse a version number, because V2's + * global UNIQUE constraints are untouched. A dedicated follow-up makes both candidate-scoped. + */ + @Test + void profileVersionAndActiveSlotRemainGloballyUnique() throws Exception { + String url = migrate("v14globaluniqueness"); + try (Connection connection = DriverManager.getConnection(url, "sa", ""); + Statement statement = connection.createStatement()) { + long owner = defaultCandidateId(statement); + statement.executeUpdate(candidate("second-candidate")); + long other = candidateId(statement, "second-candidate"); + statement.executeUpdate(ownedProfile(1, true, owner)); + + assertThatThrownBy(() -> statement.executeUpdate(ownedProfile(1, false, other))) + .as("version 1 for another candidate").isInstanceOf(SQLException.class); + assertThatThrownBy(() -> statement.executeUpdate(ownedProfile(2, true, other))) + .as("second active profile").isInstanceOf(SQLException.class); + + statement.executeUpdate(ownedProfile(2, false, other)); + } + } + + private String migrate(String name) { + String url = "jdbc:h2:mem:" + name + ";MODE=PostgreSQL;DB_CLOSE_DELAY=-1;DATABASE_TO_LOWER=TRUE"; + Flyway.configure().dataSource(url, "sa", "").load().migrate(); + return url; + } + + private long defaultCandidateId(Statement statement) throws Exception { + return candidateId(statement, "default"); + } + + private long candidateId(Statement statement, String stableKey) throws Exception { + try (var rows = statement.executeQuery( + "SELECT id FROM candidates WHERE stable_key = '" + stableKey + "'")) { + assertThat(rows.next()).isTrue(); + return rows.getLong("id"); + } + } + + private String candidate(String stableKey) { + return "INSERT INTO candidates (stable_key, created_at, updated_at) VALUES ('" + + stableKey + "', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)"; + } + + private String profile(int version, boolean active) { + return profileColumns("", version, active, ""); + } + + private String ownedProfile(int version, boolean active, long candidateId) { + return profileColumns("candidate_id, ", version, active, candidateId + ", "); + } + + private String profileColumns(String ownerColumn, int version, boolean active, String ownerValue) { + return """ + INSERT INTO candidate_profiles (%sprofile_version, full_name, location, + education_institution, degree, study_start_year, current_student, + final_year_student, commercial_java_experience_years, source_hash, + created_at, updated_at, active, active_slot) + VALUES (%s%d, 'Stored Candidate', 'Bucharest, Romania', 'Stored University', + 'BSc', 2025, TRUE, FALSE, 0, 'hash-%d', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP, + %s, %s) + """.formatted(ownerColumn, ownerValue, version, version, + active ? "TRUE" : "FALSE", active ? "1" : "NULL"); + } +} diff --git a/src/test/java/com/jobpilot/candidate/service/CandidateProfileBootstrapIntegrationTest.java b/src/test/java/com/jobpilot/candidate/service/CandidateProfileBootstrapIntegrationTest.java index 13dd440..0dc7a8a 100644 --- a/src/test/java/com/jobpilot/candidate/service/CandidateProfileBootstrapIntegrationTest.java +++ b/src/test/java/com/jobpilot/candidate/service/CandidateProfileBootstrapIntegrationTest.java @@ -4,8 +4,12 @@ import static org.assertj.core.api.Assertions.assertThat; import com.jobpilot.candidate.config.CandidateProfileProperties; +import com.jobpilot.candidate.domain.Candidate; import com.jobpilot.candidate.domain.CandidateProfile; import com.jobpilot.candidate.repository.CandidateProfileRepository; +import com.jobpilot.candidate.repository.CandidateRepository; +import java.math.BigDecimal; +import java.time.Instant; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; @@ -21,11 +25,15 @@ }) @Transactional class CandidateProfileBootstrapIntegrationTest { + private static final Instant NOW = Instant.parse("2026-07-19T00:00:00Z"); + @Autowired private CandidateProfileBootstrapService bootstrap; @Autowired private CandidateProfileRepository profiles; @Autowired + private CandidateRepository candidates; + @Autowired private CandidateProfileProperties configuredProfile; @Test @@ -44,6 +52,57 @@ void validProfileIsBootstrappedAndRepeatedBootstrapIsIdempotent() { assertThat(active.getProjects()).hasSize(4); } + @Test + void bootstrappedProfileIsOwnedByTheConfiguredCandidate() { + CandidateProfile active = profiles.findByActiveTrue().orElseThrow(); + Candidate owner = candidates.findByStableKey(configuredProfile.candidateKey()).orElseThrow(); + + assertThat(active.getCandidate().getId()).isEqualTo(owner.getId()); + assertThat(owner.getStableKey()).isEqualTo("default"); + assertThat(profiles.findByCandidateIdAndActiveTrue(owner.getId())) + .contains(active); + assertThat(profiles.findByCandidateIdAndProfileVersion(owner.getId(), 1)) + .contains(active); + } + + /** The migration seeds the owner, so bootstrap resolves it instead of adding another. */ + @Test + void repeatedBootstrapReusesTheSeededCandidateRow() { + long before = candidates.count(); + + bootstrap.bootstrap(configuredProfile); + bootstrap.bootstrap(withVersion(configuredProfile, 2)); + + assertThat(candidates.count()).isEqualTo(before); + assertThat(candidates.findAll()).extracting(Candidate::getStableKey) + .containsExactly("default"); + assertThat(profiles.findAll()).extracting(profile -> profile.getCandidate().getId()) + .containsOnly(candidates.findByStableKey("default").orElseThrow().getId()); + } + + /** + * Candidate-scoped lookups must not answer with another candidate's profile. The second + * candidate's version is stored inactive because profile_version and active_slot are still + * globally unique in the schema: one active profile per installation is exactly the + * single-candidate limitation this PR does not yet lift. + */ + @Test + void candidateScopedLookupsDoNotCrossCandidates() { + Candidate owner = candidates.findByStableKey("default").orElseThrow(); + CandidateProfile configured = profiles.findByCandidateIdAndActiveTrue(owner.getId()).orElseThrow(); + Candidate other = candidates.saveAndFlush(new Candidate("second-candidate", NOW)); + CandidateProfile otherProfile = profiles.saveAndFlush(new CandidateProfile(other, 2, + "Other Candidate", "Bucharest, Romania", "Other University", "BSc", 2025, null, + true, false, BigDecimal.ZERO, "other-source-hash", NOW, false)); + + assertThat(profiles.findByCandidateIdAndProfileVersion(owner.getId(), 2)).isEmpty(); + assertThat(profiles.findByCandidateIdAndProfileVersion(other.getId(), 2)) + .contains(otherProfile); + assertThat(profiles.findByCandidateIdAndProfileVersion(other.getId(), 1)).isEmpty(); + assertThat(profiles.findByCandidateIdAndActiveTrue(other.getId())).isEmpty(); + assertThat(profiles.findByCandidateIdAndActiveTrue(owner.getId())).contains(configured); + } + @Test void higherVersionCreatesNewActiveVersionAndPreservesPreviousFacts() { CandidateProfile previous = profiles.findByActiveTrue().orElseThrow(); diff --git a/src/test/java/com/jobpilot/candidate/service/CandidateProfileBootstrapServiceTest.java b/src/test/java/com/jobpilot/candidate/service/CandidateProfileBootstrapServiceTest.java index d71b191..fe124f5 100644 --- a/src/test/java/com/jobpilot/candidate/service/CandidateProfileBootstrapServiceTest.java +++ b/src/test/java/com/jobpilot/candidate/service/CandidateProfileBootstrapServiceTest.java @@ -2,8 +2,11 @@ import static com.jobpilot.candidate.CandidateProfileTestData.skill; import static com.jobpilot.candidate.CandidateProfileTestData.validProfile; +import static com.jobpilot.candidate.CandidateProfileTestData.withCandidateKey; import static com.jobpilot.candidate.CandidateProfileTestData.withSkills; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -11,8 +14,11 @@ import static org.mockito.Mockito.when; import com.fasterxml.jackson.databind.ObjectMapper; +import com.jobpilot.candidate.config.CandidateProfileProperties; +import com.jobpilot.candidate.domain.Candidate; import com.jobpilot.candidate.domain.CandidateProfile; import com.jobpilot.candidate.repository.CandidateProfileRepository; +import com.jobpilot.candidate.repository.CandidateRepository; import java.math.BigDecimal; import jakarta.validation.Validation; import java.time.Clock; @@ -21,15 +27,17 @@ import java.util.List; import java.util.Optional; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.springframework.test.util.ReflectionTestUtils; class CandidateProfileBootstrapServiceTest { + private static final Instant NOW = Instant.parse("2026-07-19T00:00:00Z"); + @Test void invalidProfileFailsBeforePersistence() { CandidateProfileRepository repository = mock(CandidateProfileRepository.class); - var validator = new CandidateProfileDefinitionValidator( - Validation.buildDefaultValidatorFactory().getValidator()); - var service = new CandidateProfileBootstrapService(repository, validator, - new ObjectMapper(), Clock.fixed(Instant.parse("2026-07-19T00:00:00Z"), ZoneOffset.UTC)); + CandidateRepository candidates = mock(CandidateRepository.class); + var service = service(repository, candidates); var invalid = withSkills(validProfile(2), List.of( skill("java", "java", 0), skill("java", "spring", 1))); @@ -37,23 +45,65 @@ void invalidProfileFailsBeforePersistence() { .isInstanceOf(CandidateProfileValidationException.class) .hasMessageContaining("duplicate stable skill key"); verifyNoInteractions(repository); + verifyNoInteractions(candidates); } @Test void changedFactsCannotOverwriteAnExistingProfileVersion() { CandidateProfileRepository repository = mock(CandidateProfileRepository.class); - CandidateProfile stored = new CandidateProfile(1, "Stored Candidate", "Bucharest, Romania", - "Stored University", "BSc", 2025, null, true, false, BigDecimal.ZERO, - "stored-source-hash", Instant.parse("2026-07-19T00:00:00Z"), true); - when(repository.findByActiveTrue()).thenReturn(Optional.of(stored)); - var validator = new CandidateProfileDefinitionValidator( - Validation.buildDefaultValidatorFactory().getValidator()); - var service = new CandidateProfileBootstrapService(repository, validator, - new ObjectMapper(), Clock.fixed(Instant.parse("2026-07-19T00:00:00Z"), ZoneOffset.UTC)); + CandidateRepository candidates = mock(CandidateRepository.class); + Candidate candidate = candidate("default", 7L); + CandidateProfile stored = new CandidateProfile(candidate, 1, "Stored Candidate", + "Bucharest, Romania", "Stored University", "BSc", 2025, null, true, false, + BigDecimal.ZERO, "stored-source-hash", NOW, true); + when(candidates.findByStableKey("default")).thenReturn(Optional.of(candidate)); + when(repository.findByCandidateIdAndActiveTrue(7L)).thenReturn(Optional.of(stored)); + var service = service(repository, candidates); assertThatThrownBy(() -> service.bootstrap(validProfile(1))) .isInstanceOf(CandidateProfileVersionConflictException.class) .hasMessageContaining("facts changed without increasing profileVersion"); - verify(repository, never()).saveAndFlush(org.mockito.ArgumentMatchers.any()); + verify(repository, never()).saveAndFlush(any()); + } + + /** + * The source hash fingerprints candidate facts. Ownership is not one of them, so re-keying + * the owner must not make an already stored profile version look changed -- which is exactly + * what would strand an existing database on "facts changed without increasing profileVersion". + */ + @Test + void owningCandidateIsNotPartOfTheProfileFingerprint() { + assertThat(bootstrappedSourceHash(withCandidateKey(validProfile(1), "second-candidate"))) + .isEqualTo(bootstrappedSourceHash(validProfile(1))); + } + + private String bootstrappedSourceHash(CandidateProfileProperties definition) { + CandidateProfileRepository repository = mock(CandidateProfileRepository.class); + CandidateRepository candidates = mock(CandidateRepository.class); + when(candidates.findByStableKey(definition.candidateKey())) + .thenReturn(Optional.of(candidate(definition.candidateKey(), 7L))); + when(repository.findByCandidateIdAndActiveTrue(7L)).thenReturn(Optional.empty()); + when(repository.findByCandidateIdAndProfileVersion(7L, definition.profileVersion())) + .thenReturn(Optional.empty()); + + service(repository, candidates).bootstrap(definition); + + ArgumentCaptor saved = ArgumentCaptor.forClass(CandidateProfile.class); + verify(repository).saveAndFlush(saved.capture()); + return saved.getValue().getSourceHash(); + } + + private CandidateProfileBootstrapService service(CandidateProfileRepository repository, + CandidateRepository candidates) { + var validator = new CandidateProfileDefinitionValidator( + Validation.buildDefaultValidatorFactory().getValidator()); + return new CandidateProfileBootstrapService(repository, candidates, validator, + new ObjectMapper(), Clock.fixed(NOW, ZoneOffset.UTC)); + } + + private Candidate candidate(String stableKey, long id) { + Candidate candidate = new Candidate(stableKey, NOW); + ReflectionTestUtils.setField(candidate, "id", id); + return candidate; } } diff --git a/src/test/java/com/jobpilot/jobs/PostgresPersistenceIT.java b/src/test/java/com/jobpilot/jobs/PostgresPersistenceIT.java index 991ff06..23da840 100644 --- a/src/test/java/com/jobpilot/jobs/PostgresPersistenceIT.java +++ b/src/test/java/com/jobpilot/jobs/PostgresPersistenceIT.java @@ -16,6 +16,7 @@ import com.jobpilot.candidate.repository.CandidateProfileRepository; import com.jobpilot.candidate.repository.CandidateProjectBulletRepository; import com.jobpilot.candidate.repository.CandidateProjectRepository; +import com.jobpilot.candidate.repository.CandidateRepository; import com.jobpilot.candidate.repository.CandidateSkillRepository; import com.jobpilot.config.JobPilotProperties; import com.jobpilot.jobs.domain.Job; @@ -127,6 +128,8 @@ static void datasource(DynamicPropertyRegistry registry) { @Autowired private JdbcTemplate jdbcTemplate; @Autowired + private CandidateRepository candidates; + @Autowired private CandidateProfileRepository candidateProfiles; @Autowired private CandidateSkillRepository candidateSkills; @@ -832,7 +835,9 @@ private Job job(String externalId, String url, Instant seenAt) { } private CandidateProfile profile(int version, boolean active) { - return new CandidateProfile(version, "Test Candidate", "Bucharest, Romania", + // Owned by the candidate V14 seeds; a profile cannot exist without an owner. + return new CandidateProfile(candidates.findByStableKey("default").orElseThrow(), + version, "Test Candidate", "Bucharest, Romania", "Test University", "BSc in Informatics", 2025, null, true, false, BigDecimal.ZERO, Integer.toHexString(version).repeat(16), Instant.parse("2026-07-19T00:00:00Z"), active);