fix: read the public projection by the columns it actually has
Deleting a working copy returned 500. The reference check queried `public_resource_projection.document_id`, and that column does not exist — the table addresses records by `(resource_type, resource_id)` because one table holds cases, questions, projects and releases alike. Five of the six columns in that query were verified against the migrations; this one was assumed, and it was the one that was wrong. It also has no foreign key to `document`, so it was never going to block a delete the way the check implied. What it can do is outlive the record: the projection is derived data with nothing to cascade it away, and a row left behind points the public site at something that is gone. So publication is now checked directly on the projection as well as on `workflow_status` — the two live in different tables and can disagree — and a withdrawn projection is removed with the record, which cascades its public routes. The real failure was that this SQL had never run. The neighbouring integration test says so in its own header: the standard `check` does not start Testcontainers, so persistence SQL passes the build without ever being executed, and neither compilation nor a unit test catches a column name. The delete path was simply outside it. It has its own task now, and eight scenarios that run against real PostgreSQL — including the exact query that failed.
This commit is contained in:
DongHyeonka
@@ -138,6 +138,14 @@ def postgresqlTechLogPublicPersistenceIntegrationTest = registerPostgreSqlReadin
|
||||
'postgresqlTechLogPublicPersistenceIntegrationTest',
|
||||
'dev.caskeleton.adapter.outbound.persistence.techlog.publicsite.PublicSitePersistenceIntegrationTest')
|
||||
|
||||
// studio-management-v1: 작업본 삭제 SQL 을 실제 PostgreSQL 위에서 돌린다. 이 태스크는 사고
|
||||
// 하나에서 나왔다 — 참조 검사가 없는 열(public_resource_projection.document_id)을 읽었고,
|
||||
// 컴파일과 단위 테스트를 모두 통과한 뒤 작성자가 삭제를 누른 순간 500 이 됐다. 위 둘과 같은
|
||||
// 이유이며, 삭제 경로만 그 밖에 있었다.
|
||||
def postgresqlTechLogManagementPersistenceIntegrationTest = registerPostgreSqlReadinessTest(
|
||||
'postgresqlTechLogManagementPersistenceIntegrationTest',
|
||||
'dev.caskeleton.adapter.outbound.persistence.techlog.management.ManagementPersistenceIntegrationTest')
|
||||
|
||||
def verifyJpaSqlConstructionSafety = tasks.register('verifyJpaSqlConstructionSafety') {
|
||||
group = 'verification'
|
||||
description = 'Rejects concatenated SQL construction and non-parameterized PostgreSQL timeout configuration.'
|
||||
|
||||
+29
-1
@@ -68,13 +68,41 @@ public class JdbcDocumentDeletionAdapter implements DocumentDeletionPort {
|
||||
+ " UNION ALL SELECT 1 FROM project_document_link WHERE document_id = :id"
|
||||
+ " UNION ALL SELECT 1 FROM topic_featured_document WHERE document_id = :id"
|
||||
+ " UNION ALL SELECT 1 FROM project_decision WHERE source_case_id = :id"
|
||||
+ " UNION ALL SELECT 1 FROM public_resource_projection WHERE document_id = :id"
|
||||
+ ")")
|
||||
.param("id", id)
|
||||
.query(Boolean.class)
|
||||
.single());
|
||||
}
|
||||
|
||||
/**
|
||||
* 공개 투영은 {@code document_id} 가 아니라 {@code (resource_type, resource_id)} 로 기록을 가리킨다 — 한 테이블이
|
||||
* Case·Question·Project·Release 를 모두 담기 때문이다. 외래키도 없다.
|
||||
*/
|
||||
@Override
|
||||
public boolean publiclyProjected(String resourceType, UUID id) {
|
||||
return Boolean.TRUE.equals(
|
||||
jdbcClient
|
||||
.sql(
|
||||
"SELECT EXISTS (SELECT 1 FROM public_resource_projection"
|
||||
+ " WHERE resource_type = :type AND resource_id = :id"
|
||||
+ " AND publication_state = 'ACTIVE')")
|
||||
.param("type", resourceType)
|
||||
.param("id", id)
|
||||
.query(Boolean.class)
|
||||
.single());
|
||||
}
|
||||
|
||||
@Override
|
||||
public int deleteProjection(String resourceType, UUID id) {
|
||||
return jdbcClient
|
||||
.sql(
|
||||
"DELETE FROM public_resource_projection"
|
||||
+ " WHERE resource_type = :type AND resource_id = :id")
|
||||
.param("type", resourceType)
|
||||
.param("id", id)
|
||||
.update();
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean questionReferenced(UUID id) {
|
||||
return Boolean.TRUE.equals(
|
||||
|
||||
+235
@@ -0,0 +1,235 @@
|
||||
package dev.caskeleton.adapter.outbound.persistence.techlog.management;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
|
||||
import com.zaxxer.hikari.HikariConfig;
|
||||
import com.zaxxer.hikari.HikariDataSource;
|
||||
import java.util.UUID;
|
||||
import org.flywaydb.core.Flyway;
|
||||
import org.junit.jupiter.api.AfterAll;
|
||||
import org.junit.jupiter.api.BeforeAll;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.springframework.jdbc.core.simple.JdbcClient;
|
||||
import org.testcontainers.DockerClientFactory;
|
||||
import org.testcontainers.postgresql.PostgreSQLContainer;
|
||||
|
||||
/**
|
||||
* 작업본 삭제 SQL 을 실제 PostgreSQL 위에서 돌린다.
|
||||
*
|
||||
* <p>이 파일은 사고 하나에서 나왔다. {@code documentReferenced} 가 {@code public_resource_projection.document_id}
|
||||
* 를 읽었는데 그 열은 없다 — 그 테이블은 {@code (resource_type, resource_id)} 로 기록을 가리킨다. 컴파일도 단위 테스트도 통과했고, 작성자가
|
||||
* 삭제를 누른 순간 500 이 됐다. 옆 패키지의 통합 테스트가 그 위험을 정확히 예고하고 있었는데 (<i>"컴파일도 단위 테스트도 컬럼 이름 오타를 검증하지 못한다"</i>)
|
||||
* 삭제 경로만 그 밖에 있었다.
|
||||
*
|
||||
* <p>그래서 여기서 겨냥하는 것은 결과값이 아니라 <b>SQL 이 실행되는가</b> 다. 열 이름, 조인 조건, 그리고 CASCADE 가 실제로 어디까지 따라오는지.
|
||||
*/
|
||||
class ManagementPersistenceIntegrationTest {
|
||||
|
||||
private static final String IMAGE =
|
||||
System.getProperty("jpa.evidence.postgresql.image", "postgres:16-alpine");
|
||||
|
||||
private static PostgreSQLContainer postgres;
|
||||
private static HikariDataSource dataSource;
|
||||
private static JdbcClient jdbcClient;
|
||||
private static JdbcDocumentDeletionAdapter deletion;
|
||||
|
||||
@BeforeAll
|
||||
static void migrate() {
|
||||
if (!DockerClientFactory.instance().isDockerAvailable()) {
|
||||
throw new IllegalStateException(
|
||||
"Docker is required for the management persistence integration test;"
|
||||
+ " skipping is forbidden");
|
||||
}
|
||||
postgres = new PostgreSQLContainer(IMAGE).withReuse(false);
|
||||
postgres.start();
|
||||
|
||||
HikariConfig config = new HikariConfig();
|
||||
config.setJdbcUrl(postgres.getJdbcUrl());
|
||||
config.setUsername(postgres.getUsername());
|
||||
config.setPassword(postgres.getPassword());
|
||||
config.setMaximumPoolSize(5);
|
||||
config.setMinimumIdle(1);
|
||||
dataSource = new HikariDataSource(config);
|
||||
|
||||
Flyway.configure()
|
||||
.dataSource(dataSource)
|
||||
.locations("classpath:db/migration/postgresql")
|
||||
.baselineOnMigrate(false)
|
||||
.outOfOrder(false)
|
||||
.load()
|
||||
.migrate();
|
||||
|
||||
jdbcClient = JdbcClient.create(dataSource);
|
||||
deletion = new JdbcDocumentDeletionAdapter(jdbcClient);
|
||||
}
|
||||
|
||||
@AfterAll
|
||||
static void stopPostgreSql() {
|
||||
if (dataSource != null) {
|
||||
dataSource.close();
|
||||
}
|
||||
if (postgres != null) {
|
||||
postgres.stop();
|
||||
}
|
||||
}
|
||||
|
||||
private static UUID insertCase(String title) {
|
||||
UUID id = UUID.randomUUID();
|
||||
jdbcClient
|
||||
.sql(
|
||||
"INSERT INTO document (id, document_type, title, created_by, updated_by)"
|
||||
+ " VALUES (:id, 'CASE', :title, 'test', 'test')")
|
||||
.param("id", id)
|
||||
.param("title", title)
|
||||
.update();
|
||||
jdbcClient.sql("INSERT INTO case_detail (document_id) VALUES (:id)").param("id", id).update();
|
||||
return id;
|
||||
}
|
||||
|
||||
private static UUID insertQuestion(String question) {
|
||||
UUID id = UUID.randomUUID();
|
||||
jdbcClient
|
||||
.sql(
|
||||
"INSERT INTO open_question (id, question, created_by, updated_by)"
|
||||
+ " VALUES (:id, :question, 'test', 'test')")
|
||||
.param("id", id)
|
||||
.param("question", question)
|
||||
.update();
|
||||
return id;
|
||||
}
|
||||
|
||||
/** 이 테이블은 NOT NULL 이 많다 — 참조 검사에 필요한 열만으로는 행을 만들 수 없다. */
|
||||
private static void insertProjection(String resourceType, UUID id, String state, String title) {
|
||||
jdbcClient
|
||||
.sql(
|
||||
"INSERT INTO public_resource_projection (resource_type, resource_id, source_version,"
|
||||
+ " publication_state, visibility, title, payload_schema_version, payload,"
|
||||
+ " content_hash, published_at, updated_at, navigation_path)"
|
||||
+ " VALUES (:type, :id, 0, :state, 'PUBLIC', :title, 1, '{}'::jsonb,"
|
||||
+ " :hash, now(), now(), :path)")
|
||||
.param("type", resourceType)
|
||||
.param("id", id)
|
||||
.param("state", state)
|
||||
.param("title", title)
|
||||
.param("hash", "0".repeat(64))
|
||||
.param("path", "/test/" + id)
|
||||
.update();
|
||||
}
|
||||
|
||||
private static int countIn(String table, String column, UUID id) {
|
||||
return jdbcClient
|
||||
.sql("SELECT COUNT(*) FROM " + table + " WHERE " + column + " = :id")
|
||||
.param("id", id)
|
||||
.query(Integer.class)
|
||||
.single();
|
||||
}
|
||||
|
||||
@Test
|
||||
void findsADocumentOnlyUnderItsOwnType() {
|
||||
UUID id = insertCase("타입이 맞아야 찾힌다");
|
||||
|
||||
assertThat(deletion.findDocument(id, "CASE")).isPresent();
|
||||
// Case 주소로 Reference 를 지울 수 없어야 한다 — 한 테이블을 나눠 쓰기 때문에 이것이 유일한 방어다.
|
||||
assertThat(deletion.findDocument(id, "REFERENCE")).isEmpty();
|
||||
}
|
||||
|
||||
@Test
|
||||
void reportsNoReferencesForAFreshDraft() {
|
||||
// 작성자가 방금 연 초안. 이 경로가 막히면 기능 자체가 무의미해진다.
|
||||
assertThat(deletion.documentReferenced(insertCase("갓 만든 초안"))).isFalse();
|
||||
}
|
||||
|
||||
@Test
|
||||
void reportsEveryNonCascadingReferrer() {
|
||||
UUID target = insertCase("가리켜지는 쪽");
|
||||
UUID source = insertCase("가리키는 쪽");
|
||||
jdbcClient
|
||||
.sql(
|
||||
"INSERT INTO document_relation (source_document_id, target_document_id, relation_type)"
|
||||
+ " VALUES (:source, :target, 'RELATED')")
|
||||
.param("source", source)
|
||||
.param("target", target)
|
||||
.update();
|
||||
|
||||
assertThat(deletion.documentReferenced(target)).isTrue();
|
||||
// 나가는 관계는 자기 것이라 CASCADE 로 따라간다 — 가리키는 쪽은 자유롭게 지울 수 있어야 한다.
|
||||
assertThat(deletion.documentReferenced(source)).isFalse();
|
||||
}
|
||||
|
||||
@Test
|
||||
void readsThePublicProjectionByResourceTypeAndId() {
|
||||
// 이 테스트가 존재하는 이유. 이 쿼리가 열 이름을 틀렸고 아무도 잡지 못했다.
|
||||
UUID id = insertCase("공개된 기록");
|
||||
assertThat(deletion.publiclyProjected("CASE", id)).isFalse();
|
||||
|
||||
insertProjection("CASE", id, "ACTIVE", "공개된 기록");
|
||||
|
||||
assertThat(deletion.publiclyProjected("CASE", id)).isTrue();
|
||||
assertThat(deletion.publiclyProjected("QUESTION", id)).isFalse();
|
||||
}
|
||||
|
||||
@Test
|
||||
void deletingCarriesTheDocumentsOwnRowsAndItsWithdrawnProjection() {
|
||||
UUID id = insertCase("지워질 기록");
|
||||
insertProjection("CASE", id, "WITHDRAWN", "지워질 기록");
|
||||
jdbcClient
|
||||
.sql(
|
||||
"INSERT INTO public_route (resource_type, slug, resource_id, route_role)"
|
||||
+ " VALUES ('CASE', :slug, :id, 'CANONICAL')")
|
||||
.param("slug", "deleted-" + id)
|
||||
.param("id", id)
|
||||
.update();
|
||||
|
||||
assertThat(deletion.deleteProjection("CASE", id)).isEqualTo(1);
|
||||
// 공개 경로는 투영을 CASCADE 로 따라간다 — 손으로 지우지 않는 것이 옳은지 여기서 확인한다.
|
||||
assertThat(countIn("public_route", "resource_id", id)).isZero();
|
||||
|
||||
assertThat(deletion.deleteDocument(id, 0L)).isEqualTo(1);
|
||||
assertThat(countIn("case_detail", "document_id", id)).isZero();
|
||||
assertThat(deletion.findDocument(id, "CASE")).isEmpty();
|
||||
}
|
||||
|
||||
@Test
|
||||
void refusesToDeleteWhenTheExpectedVersionHasMoved() {
|
||||
UUID id = insertCase("먼저 수정된 기록");
|
||||
jdbcClient.sql("UPDATE document SET version = 3 WHERE id = :id").param("id", id).update();
|
||||
|
||||
assertThat(deletion.deleteDocument(id, 0L)).isZero();
|
||||
assertThat(deletion.findDocument(id, "CASE")).isPresent();
|
||||
}
|
||||
|
||||
@Test
|
||||
void readsQuestionReferencesFromItsOwnTables() {
|
||||
UUID id = insertQuestion("질문은 다른 테이블이다");
|
||||
assertThat(deletion.findQuestion(id)).isPresent();
|
||||
assertThat(deletion.questionReferenced(id)).isFalse();
|
||||
|
||||
// 이 테이블은 CHECK 로 id 가 고정된 단일 행이다 — 테스트마다 새로 넣을 수 없다.
|
||||
jdbcClient
|
||||
.sql(
|
||||
"INSERT INTO home_focus_config (id, open_question_id, created_by, updated_by)"
|
||||
+ " VALUES (:configId, :id, 'test', 'test')"
|
||||
+ " ON CONFLICT (id) DO UPDATE SET open_question_id = EXCLUDED.open_question_id")
|
||||
.param("configId", UUID.fromString("00000000-0000-0000-0000-000000000003"))
|
||||
.param("id", id)
|
||||
.update();
|
||||
|
||||
assertThat(deletion.questionReferenced(id)).isTrue();
|
||||
}
|
||||
|
||||
@Test
|
||||
void deletesAQuestionAndItsOwnChildren() {
|
||||
UUID id = insertQuestion("지워질 질문");
|
||||
jdbcClient
|
||||
.sql(
|
||||
"INSERT INTO question_point (id, question_id, point_kind, content, display_order)"
|
||||
+ " VALUES (:pointId, :id, 'ASSUMPTION', '가설', 0)")
|
||||
.param("pointId", UUID.randomUUID())
|
||||
.param("id", id)
|
||||
.update();
|
||||
|
||||
assertThat(deletion.deleteQuestion(id, 0L)).isEqualTo(1);
|
||||
assertThat(countIn("question_point", "question_id", id)).isZero();
|
||||
assertThat(deletion.findQuestion(id)).isEmpty();
|
||||
}
|
||||
}
|
||||
+12
@@ -27,6 +27,18 @@ public interface DocumentDeletionPort {
|
||||
*/
|
||||
boolean documentReferenced(UUID id);
|
||||
|
||||
/**
|
||||
* 공개 투영이 아직 살아 있는지. {@code workflow_status} 와 별개로 확인한다 — 둘은 다른 테이블에 있고, 어긋난 상태로 지우면 공개 화면이 없는 기록을
|
||||
* 가리키게 된다.
|
||||
*/
|
||||
boolean publiclyProjected(String resourceType, UUID id);
|
||||
|
||||
/**
|
||||
* 내려간 공개 투영을 치운다. 투영은 기록에서 파생된 것이라 기록과 함께 사라져야 하고, 외래키가 없어 DB 가 대신 해 주지 않는다. 공개 경로는 투영을 CASCADE 로
|
||||
* 따라간다.
|
||||
*/
|
||||
int deleteProjection(String resourceType, UUID id);
|
||||
|
||||
Optional<DeletableQuestion> findQuestion(UUID id);
|
||||
|
||||
int deleteQuestion(UUID id, long expectedVersion);
|
||||
|
||||
+5
-1
@@ -49,7 +49,8 @@ public class DeleteDocumentDraftUseCase {
|
||||
() ->
|
||||
ManagementException.of(
|
||||
ManagementError.DOCUMENT_NOT_FOUND, "no such working copy"));
|
||||
if ("PUBLISHED".equals(current.workflowStatus())) {
|
||||
if ("PUBLISHED".equals(current.workflowStatus())
|
||||
|| documents.publiclyProjected(documentType, command.id())) {
|
||||
throw ManagementException.of(
|
||||
ManagementError.DOCUMENT_PUBLISHED,
|
||||
"the record is published; unpublish it before deleting");
|
||||
@@ -59,6 +60,9 @@ public class DeleteDocumentDraftUseCase {
|
||||
ManagementError.DOCUMENT_IN_USE,
|
||||
"another record still links to this one; unlink it first");
|
||||
}
|
||||
// 내려간 공개 투영은 기록에서 파생된 것이므로 함께 치운다 — 외래키가 없어 DB 가
|
||||
// 대신 해 주지 않고, 남겨 두면 없는 기록을 가리키는 행이 된다.
|
||||
documents.deleteProjection(documentType, command.id());
|
||||
if (documents.deleteDocument(command.id(), command.expectedVersion()) == 0) {
|
||||
throw ManagementException.withDetails(
|
||||
ManagementError.VERSION_CONFLICT,
|
||||
|
||||
+6
@@ -43,11 +43,17 @@ public class DeleteQuestionUseCase {
|
||||
() ->
|
||||
ManagementException.of(
|
||||
ManagementError.QUESTION_NOT_FOUND, "no such question"));
|
||||
if (documents.publiclyProjected("QUESTION", command.id())) {
|
||||
throw ManagementException.of(
|
||||
ManagementError.DOCUMENT_PUBLISHED,
|
||||
"the question is published; unpublish it before deleting");
|
||||
}
|
||||
if (documents.questionReferenced(command.id())) {
|
||||
throw ManagementException.of(
|
||||
ManagementError.QUESTION_IN_USE,
|
||||
"another record still links to this question; unlink it first");
|
||||
}
|
||||
documents.deleteProjection("QUESTION", command.id());
|
||||
if (documents.deleteQuestion(command.id(), command.expectedVersion()) == 0) {
|
||||
throw ManagementException.withDetails(
|
||||
ManagementError.VERSION_CONFLICT,
|
||||
|
||||
Reference in New Issue
Block a user