Commit @4280ec60720e26e31d1d3be5edd5581a9323fe13

ITN Dev 07-22

fix: 잘못된 요청을 500 대신 4xx로 응답하고 부분 실패 채널생성 경계 테스트 추가

- GlobalExceptionHandler(@RestControllerAdvice) 추가: IllegalArgumentException은
  404, SeedParseException은 400으로 매핑하고 예외 메시지를 ApiError로 그대로
  돌려준다. Exception 전체를 잡는 catch-all은 두지 않아 예상 밖의 진짜 버그는
  여전히 500으로 드러나게 한다.
- SeedController 업로드에 확장자(.xlsx/.xlsm) 검사를 추가해 임의의 바이트가
  Apache POI로 그대로 전달되기 전에 SeedParseException으로 걸러낸다.
- POST /api/orgs/{id}/channels의 실제 혼합 성공/실패 결과가 200으로 내려가는
  경로를 컨트롤러 경계에서 검증하는 테스트를 추가하고, 담당자 정보 누락
  테스트의 law 단정을 보강했다.

@4280ec60720e26e31d1d3be5edd5581a9323fe13

4280ec6

src/main/java/kr/itn/itnhub/config/ApiError.java (added)

+++ src/main/java/kr/itn/itnhub/config/ApiError.java

...	...	@@ -0,0 +1,8 @@
	1	+package kr.itn.itnhub.config;
	2	+
	3	+/**
	4	+ * 클라이언트 오류(4xx) 응답 본문. 사람이 읽을 수 있는 메시지 하나만 담는다.
	5	+ * 예외 자체의 메시지를 그대로 옮길 뿐 스택트레이스·SQL·파일 경로는 절대 포함하지 않는다.
	6	+ */
	7	+public record ApiError(String message) {
	8	+}

4280ec6

src/main/java/kr/itn/itnhub/config/GlobalExceptionHandler.java (added)

+++ src/main/java/kr/itn/itnhub/config/GlobalExceptionHandler.java

...	...	@@ -0,0 +1,40 @@
	1	+package kr.itn.itnhub.config;
	2	+
	3	+import kr.itn.itnhub.seed.SeedParseException;
	4	+import org.springframework.http.HttpStatus;
	5	+import org.springframework.http.ResponseEntity;
	6	+import org.springframework.web.bind.annotation.ExceptionHandler;
	7	+import org.springframework.web.bind.annotation.RestControllerAdvice;
	8	+
	9	+/**
	10	+ * 운영자의 흔한 실수를 서버 장애(500)가 아니라 4xx로 돌려준다.
	11	+ *
	12	+ * <p>{@link IllegalArgumentException}은 {@code OrganizationService.updateContact}와
	13	+ * {@code ChannelProvisionService.provision}이 존재하지 않는 기관 id에 대해 던진다 -
	14	+ * URL에 오타가 있는 것뿐이므로 404가 맞다.</p>
	15	+ *
	16	+ * <p>{@link SeedParseException}은 비어 있거나, 형식이 틀렸거나, 시트가 잘못된 업로드에
	17	+ * 대해 던진다 - 흔한 사용자 실수이므로 400과 함께 무엇이 잘못됐는지 알려줘야 한다.</p>
	18	+ *
	19	+ * <p>예외 메시지는 각 예외가 만든 그대로 내려준다(새로 지어내지 않는다) - 스택트레이스,
	20	+ * SQL, 파일 경로는 어차피 이 메시지들에 담기지 않으므로 그대로 노출해도 안전하다.
	21	+ * {@code server.error.include-message=NEVER} 기본값은 건드리지 않는다 - 여기서 직접
	22	+ * {@link ApiError} 본문을 만들어 반환하므로 그 설정과는 무관하게 동작한다.</p>
	23	+ *
	24	+ * <p><b>여기에 {@code Exception.class} catch-all을 추가하지 말 것.</b> 예상하지 못한
	25	+ * 예외까지 4xx로 감싸버리면 진짜 버그가 조용히 묻힌다. 예상 밖 예외는 기본 500 처리
	26	+ * 그대로 두는 것이 의도다.</p>
	27	+ */
	28	+@RestControllerAdvice
	29	+public class GlobalExceptionHandler {
	30	+
	31	+ @ExceptionHandler(IllegalArgumentException.class)
	32	+ public ResponseEntity<ApiError> handleNotFound(IllegalArgumentException e) {
	33	+ return ResponseEntity.status(HttpStatus.NOT_FOUND).body(new ApiError(e.getMessage()));
	34	+ }
	35	+
	36	+ @ExceptionHandler(SeedParseException.class)
	37	+ public ResponseEntity<ApiError> handleSeedParse(SeedParseException e) {
	38	+ return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(new ApiError(e.getMessage()));
	39	+ }
	40	+}

60c33c2

4280ec6

src/main/java/kr/itn/itnhub/seed/SeedController.java

--- src/main/java/kr/itn/itnhub/seed/SeedController.java

+++ src/main/java/kr/itn/itnhub/seed/SeedController.java


         if (file.isEmpty()) {
             throw new SeedParseException("업로드된 파일이 비어 있습니다.");
         }
+        // 확장자 검사만으로 파일 형식을 완전히 보장하진 못하지만, 엉뚱한 바이트를
+        // 곧장 Apache POI에 넘기기 전에 흔한 실수(다른 파일을 잘못 올림)를 걸러내고
+        // 사람이 이해할 수 있는 메시지로 알려준다.
+        String filename = file.getOriginalFilename();
+        if (filename == null || !hasAllowedExtension(filename)) {
+            throw new SeedParseException("엑셀 파일(.xlsx 또는 .xlsm)만 업로드할 수 있습니다.");
+        }
         try (InputStream in = file.getInputStream()) {
             return seedService.seed(in);
         }
     }
+
+    private static boolean hasAllowedExtension(String filename) {
+        String lower = filename.toLowerCase();
+        return lower.endsWith(".xlsx") || lower.endsWith(".xlsm");
+    }
 }

60c33c2

4280ec6

src/test/java/kr/itn/itnhub/org/OrganizationControllerTest.java

--- src/test/java/kr/itn/itnhub/org/OrganizationControllerTest.java

+++ src/test/java/kr/itn/itnhub/org/OrganizationControllerTest.java


 
 import kr.itn.itnhub.AbstractDbTest;
 import kr.itn.itnhub.mattermost.MattermostClient;
+import kr.itn.itnhub.mattermost.MattermostException;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.annotation.Autowired;

 import java.util.Optional;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.hamcrest.Matchers.containsString;
 import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.when;
 import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;

     }
 
     @Test
+    void 존재하지_않는_기관ID로_담당자정보를_수정하면_404다() throws Exception {
+        long missingId = orgId + 999999L;
+
+        mvc.perform(put("/api/orgs/{id}/contact", missingId)
+                        .with(csrf())
+                        .contentType(MediaType.APPLICATION_JSON)
+                        .content("""
+                                {
+                                  "deptName": "데이터정보화팀",
+                                  "managerName": "송민지",
+                                  "managerTitle": "과장",
+                                  "managerPhone": "02-3475-5434",
+                                  "managerEmail": "ming@arirang.com"
+                                }
+                                """))
+                .andExpect(status().isNotFound())
+                .andExpect(jsonPath("$.message").value(containsString(String.valueOf(missingId))));
+    }
+
+    @Test
     void 필수값이_비면_400이다() throws Exception {
         mvc.perform(put("/api/orgs/{id}/contact", orgId)
                         .with(csrf())

         mvc.perform(post("/api/orgs/{id}/channels", orgId).with(csrf()))
                 .andExpect(status().isOk())
                 .andExpect(jsonPath("$.mj").value("FAILED"))
-                .andExpect(jsonPath("$.message").value(
-                        org.hamcrest.Matchers.containsString("담당자 정보")));
+                .andExpect(jsonPath("$.law").value("FAILED"))
+                .andExpect(jsonPath("$.message").value(containsString("담당자 정보")));
+    }
+
+    /**
+     * Finding 2 회귀 테스트: 문정원 채널은 성공하고 법률검토 채널만 실패하는 실제 혼합
+     * 결과를 컨트롤러 경계에서 검증한다. 기존 테스트는 전부 성공 아니면(둘 다 성공)
+     * 담당자 정보 누락으로 둘 다 강제 FAILED인 경로만 다뤄, 진짜 부분 실패가 HTTP
+     * 200으로 내려가는지는 한 번도 실행되지 않았다.
+     */
+    @Test
+    void 법률검토만_실패해도_200과_부분실패_결과를_돌려준다() throws Exception {
+        Organization ready = mapper.findById(orgId);
+        ready.setDeptName("데이터정보화팀");
+        ready.setManagerName("송민지");
+        ready.setManagerTitle("과장");
+        ready.setManagerPhone("02-3475-5434");
+        ready.setManagerEmail("ming@arirang.com");
+        mapper.updateContact(ready);
+
+        when(mattermost.findChannelIdByInternalName(anyString())).thenReturn(Optional.empty());
+        when(mattermost.findChannelIdByDisplayName(anyString())).thenReturn(Optional.empty());
+        when(mattermost.createPrivateChannel(eq("org-001-mj"), anyString())).thenReturn("id-mj");
+        when(mattermost.createPrivateChannel(eq("org-001-law"), anyString()))
+                .thenThrow(new MattermostException("법률검토 서버 오류"));
+
+        mvc.perform(post("/api/orgs/{id}/channels", orgId).with(csrf()))
+                .andExpect(status().isOk())
+                .andExpect(jsonPath("$.mj").value("CREATED"))
+                .andExpect(jsonPath("$.law").value("FAILED"))
+                .andExpect(jsonPath("$.message").value(containsString("법률검토")));
     }
 }

4280ec6

src/test/java/kr/itn/itnhub/seed/SeedControllerTest.java (added)

+++ src/test/java/kr/itn/itnhub/seed/SeedControllerTest.java

...	...	@@ -0,0 +1,77 @@
	1	+package kr.itn.itnhub.seed;
	2	+
	3	+import kr.itn.itnhub.AbstractDbTest;
	4	+import org.junit.jupiter.api.BeforeEach;
	5	+import org.junit.jupiter.api.Test;
	6	+import org.springframework.beans.factory.annotation.Autowired;
	7	+import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
	8	+import org.springframework.jdbc.core.JdbcTemplate;
	9	+import org.springframework.mock.web.MockMultipartFile;
	10	+import org.springframework.security.test.context.support.WithMockUser;
	11	+import org.springframework.test.web.servlet.MockMvc;
	12	+
	13	+import static org.hamcrest.Matchers.containsString;
	14	+import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf;
	15	+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.multipart;
	16	+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
	17	+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
	18	+
	19	+/**
	20	+ * Finding 1 회귀 테스트: 업로드 컨트롤러가 확장자·빈 파일 검증 실패를 SeedParseException으로
	21	+ * 던지고, GlobalExceptionHandler가 그것을 500이 아니라 400으로 내려주는지 HTTP 경계에서 확인한다.
	22	+ */
	23	+@AutoConfigureMockMvc
	24	+@WithMockUser(roles = "ADMIN")
	25	+class SeedControllerTest extends AbstractDbTest {
	26	+
	27	+ @Autowired
	28	+ MockMvc mvc;
	29	+
	30	+ @Autowired
	31	+ JdbcTemplate jdbc;
	32	+
	33	+ @BeforeEach
	34	+ void clean() {
	35	+ jdbc.update("delete from organization");
	36	+ }
	37	+
	38	+ @Test
	39	+ void 빈_파일이면_400이다() throws Exception {
	40	+ MockMultipartFile empty = new MockMultipartFile("file", "seed.xlsx",
	41	+ "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", new byte[0]);
	42	+
	43	+ mvc.perform(multipart("/api/seed").file(empty).with(csrf()))
	44	+ .andExpect(status().isBadRequest())
	45	+ .andExpect(jsonPath("$.message").value(containsString("비어")));
	46	+ }
	47	+
	48	+ @Test
	49	+ void 확장자가_xlsx_xlsm이_아니면_400이다() throws Exception {
	50	+ MockMultipartFile txt = new MockMultipartFile("file", "seed.txt",
	51	+ "text/plain", "아무 내용".getBytes());
	52	+
	53	+ mvc.perform(multipart("/api/seed").file(txt).with(csrf()))
	54	+ .andExpect(status().isBadRequest())
	55	+ .andExpect(jsonPath("$.message").value(containsString("xlsx")));
	56	+ }
	57	+
	58	+ @Test
	59	+ void 시트가_없는_엑셀이면_400이다() throws Exception {
	60	+ byte[] wrongSheet = wrongSheetWorkbook();
	61	+ MockMultipartFile xlsx = new MockMultipartFile("file", "seed.xlsx",
	62	+ "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", wrongSheet);
	63	+
	64	+ mvc.perform(multipart("/api/seed").file(xlsx).with(csrf()))
	65	+ .andExpect(status().isBadRequest())
	66	+ .andExpect(jsonPath("$.message").value(containsString(SeedParser.SHEET_NAME)));
	67	+ }
	68	+
	69	+ private byte[] wrongSheetWorkbook() throws Exception {
	70	+ try (var wb = new org.apache.poi.xssf.usermodel.XSSFWorkbook();
	71	+ var out = new java.io.ByteArrayOutputStream()) {
	72	+ wb.createSheet("다른시트");
	73	+ wb.write(out);
	74	+ return out.toByteArray();
	75	+ }
	76	+ }
	77	+}

Add a comment

Open 0
Closed 0

List

...	...	@@ -22,8 +22,20 @@
22	22	if (file.isEmpty()) {
23	23	throw new SeedParseException("업로드된 파일이 비어 있습니다.");
24	24	}
	25	+ // 확장자 검사만으로 파일 형식을 완전히 보장하진 못하지만, 엉뚱한 바이트를
	26	+ // 곧장 Apache POI에 넘기기 전에 흔한 실수(다른 파일을 잘못 올림)를 걸러내고
	27	+ // 사람이 이해할 수 있는 메시지로 알려준다.
	28	+ String filename = file.getOriginalFilename();
	29	+ if (filename == null \|\| !hasAllowedExtension(filename)) {
	30	+ throw new SeedParseException("엑셀 파일(.xlsx 또는 .xlsm)만 업로드할 수 있습니다.");
	31	+ }
25	32	try (InputStream in = file.getInputStream()) {
26	33	return seedService.seed(in);
27	34	}
28	35	}
	36	+
	37	+ private static boolean hasAllowedExtension(String filename) {
	38	+ String lower = filename.toLowerCase();
	39	+ return lower.endsWith(".xlsx") \|\| lower.endsWith(".xlsm");
	40	+ }
29	41	}

...	...	@@ -2,6 +2,7 @@
2	2
3	3	import kr.itn.itnhub.AbstractDbTest;
4	4	import kr.itn.itnhub.mattermost.MattermostClient;
	5	+import kr.itn.itnhub.mattermost.MattermostException;
5	6	import org.junit.jupiter.api.BeforeEach;
6	7	import org.junit.jupiter.api.Test;
7	8	import org.springframework.beans.factory.annotation.Autowired;
...	...	@@ -15,7 +16,9 @@
15	16	import java.util.Optional;
16	17
17	18	import static org.assertj.core.api.Assertions.assertThat;
	19	+import static org.hamcrest.Matchers.containsString;
18	20	import static org.mockito.ArgumentMatchers.anyString;
	21	+import static org.mockito.ArgumentMatchers.eq;
19	22	import static org.mockito.Mockito.when;
20	23	import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf;
21	24	import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
...	...	@@ -85,6 +88,26 @@
85	88	}
86	89
87	90	@Test
	91	+ void 존재하지_않는_기관ID로_담당자정보를_수정하면_404다() throws Exception {
	92	+ long missingId = orgId + 999999L;
	93	+
	94	+ mvc.perform(put("/api/orgs/{id}/contact", missingId)
	95	+ .with(csrf())
	96	+ .contentType(MediaType.APPLICATION_JSON)
	97	+ .content("""
	98	+ {
	99	+ "deptName": "데이터정보화팀",
	100	+ "managerName": "송민지",
	101	+ "managerTitle": "과장",
	102	+ "managerPhone": "02-3475-5434",
	103	+ "managerEmail": "ming@arirang.com"
	104	+ }
	105	+ """))
	106	+ .andExpect(status().isNotFound())
	107	+ .andExpect(jsonPath("$.message").value(containsString(String.valueOf(missingId))));
	108	+ }
	109	+
	110	+ @Test
88	111	void 필수값이_비면_400이다() throws Exception {
89	112	mvc.perform(put("/api/orgs/{id}/contact", orgId)
90	113	.with(csrf())
...	...	@@ -125,7 +148,36 @@
125	148	mvc.perform(post("/api/orgs/{id}/channels", orgId).with(csrf()))
126	149	.andExpect(status().isOk())
127	150	.andExpect(jsonPath("$.mj").value("FAILED"))
128		- .andExpect(jsonPath("$.message").value(
129		- org.hamcrest.Matchers.containsString("담당자 정보")));
	151	+ .andExpect(jsonPath("$.law").value("FAILED"))
	152	+ .andExpect(jsonPath("$.message").value(containsString("담당자 정보")));
	153	+ }
	154	+
	155	+ /**
	156	+ * Finding 2 회귀 테스트: 문정원 채널은 성공하고 법률검토 채널만 실패하는 실제 혼합
	157	+ * 결과를 컨트롤러 경계에서 검증한다. 기존 테스트는 전부 성공 아니면(둘 다 성공)
	158	+ * 담당자 정보 누락으로 둘 다 강제 FAILED인 경로만 다뤄, 진짜 부분 실패가 HTTP
	159	+ * 200으로 내려가는지는 한 번도 실행되지 않았다.
	160	+ */
	161	+ @Test
	162	+ void 법률검토만_실패해도_200과_부분실패_결과를_돌려준다() throws Exception {
	163	+ Organization ready = mapper.findById(orgId);
	164	+ ready.setDeptName("데이터정보화팀");
	165	+ ready.setManagerName("송민지");
	166	+ ready.setManagerTitle("과장");
	167	+ ready.setManagerPhone("02-3475-5434");
	168	+ ready.setManagerEmail("ming@arirang.com");
	169	+ mapper.updateContact(ready);
	170	+
	171	+ when(mattermost.findChannelIdByInternalName(anyString())).thenReturn(Optional.empty());
	172	+ when(mattermost.findChannelIdByDisplayName(anyString())).thenReturn(Optional.empty());
	173	+ when(mattermost.createPrivateChannel(eq("org-001-mj"), anyString())).thenReturn("id-mj");
	174	+ when(mattermost.createPrivateChannel(eq("org-001-law"), anyString()))
	175	+ .thenThrow(new MattermostException("법률검토 서버 오류"));
	176	+
	177	+ mvc.perform(post("/api/orgs/{id}/channels", orgId).with(csrf()))
	178	+ .andExpect(status().isOk())
	179	+ .andExpect(jsonPath("$.mj").value("CREATED"))
	180	+ .andExpect(jsonPath("$.law").value("FAILED"))
	181	+ .andExpect(jsonPath("$.message").value(containsString("법률검토")));
130	182	}
131	183	}

Delete comment