Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat#14/organization 조회 #35

Merged
merged 9 commits into from
Aug 14, 2024
26 changes: 25 additions & 1 deletion src/docs/asciidoc/api/domain/organization/Organization.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,28 @@ include::{snippets}/organization-controller-test/create/request-fields.adoc[]
==== HTTP Response

include::{snippets}/organization-controller-test/create/http-response.adoc[]
include::{snippets}/organization-controller-test/create/response-fields-data.adoc[]
include::{snippets}/organization-controller-test/create/response-fields-data.adoc[]

=== Organization 조회 API

==== 성공

===== HTTP Request

include::{snippets}/organization-controller-test/success_find/http-request.adoc[]

===== HTTP Response

include::{snippets}/organization-controller-test/success_find/http-response.adoc[]
include::{snippets}/organization-controller-test/success_find/response-fields-data.adoc[]

==== 실패

===== HTTP Request

include::{snippets}/organization-controller-test/fail_find/http-request.adoc[]

===== HTTP Response

include::{snippets}/organization-controller-test/fail_find/http-response.adoc[]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 리뷰를 기반으로 다음과 같은 개선 사항을 제시합니다.

  1. 코드 가독성 및 유지보수성:

    • 적절한 주석 추가: 각 코드 블록이나 주요 기능에 대한 설명을 포함하여 나중에 이해하기 쉬운 구조로 만듭니다.
    • 일관된 네이밍 규칙 사용: 변수 및 메서드 이름이 일관성을 유지하도록 하여 가독성을 높입니다.
  2. 성능 최적화:

    • 응답 필드가 필요 없는 경우 데이터를 가져오지 않도록 쿼리를 최적화합니다.
    • 캐싱 전략을 검토하여 자주 조회하는 데이터에 대해 성능을 향상시킬 수 있습니다.
  3. 보안 및 데이터 검증:

    • 입력 데이터 유효성 검사 추가: 사용자 입력 값에 대한 검증 로직을 강화하여 악의적인 데이터 접근을 방지합니다.
    • 민감한 정보의 노출을 방지하기 위해 HTTP 응답에서 필요한 데이터만 반환하도록 필터링 절차를 고려합니다.

이러한 개선 사항을 반영하면 코드의 전반적인 품질이 한층 높아질 것입니다.

include::{snippets}/organization-controller-test/fail_find/response-fields.adoc[]
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@

import com.wootecam.festivals.domain.organization.dto.OrganizationCreateDto;
import com.wootecam.festivals.domain.organization.dto.OrganizationIdDto;
import com.wootecam.festivals.domain.organization.dto.OrganizationResponse;
import com.wootecam.festivals.domain.organization.service.OrganizationService;
import com.wootecam.festivals.global.api.ApiResponse;
import jakarta.validation.Valid;
import lombok.RequiredArgsConstructor;
import org.springframework.http.HttpStatus;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
Expand All @@ -28,4 +31,10 @@ public ApiResponse<OrganizationIdDto> createOrganization(

return ApiResponse.of(new OrganizationIdDto(organizationId));
}

@ResponseStatus(HttpStatus.OK)
@GetMapping("/{organizationId}")
public ApiResponse<OrganizationResponse> findOrganization(@PathVariable Long organizationId) {
return ApiResponse.of(organizationService.findOrganization(organizationId));
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 리뷰에 대한 제안은 다음과 같습니다:

  1. 가독성과 유지보수성:

    • 새로운 @GetMapping 메서드는 명확하게 이름을 부여하여 API의 목적을 분명히 해야 합니다. 예: getOrganizationById.
    • DTO 클래스와 관련된 내용을 문서화하여 사용자가 이해하기 쉽게 해야 합니다.
  2. 성능 최적화:

    • findOrganization 메서드에서 조직 ID를 조회할 때, 데이터베이스 접근을 최소화하도록 캐싱을 고려할 수 있습니다.
    • DTO 변환 또는 응답 형식으로 인한 성능 오버헤드를 줄이기 위해 객체 매핑 라이브러리 (예: MapStruct)를 사용할 수 있습니다.
  3. 보안 및 데이터 검증:

    • organizationId에 대한 추가 유효성 검사를 수행하여 존재하지 않는 ID에 대한 요청을 방지해야 합니다.
    • API 응답에 민감 정보가 포함되지 않도록 결과 DTO에 대한 필드 검토를 수행해야 합니다.

이러한 점들을 개선하면 코드의 품질과 안정성을 높일 수 있습니다.

Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.wootecam.festivals.domain.organization.dto;

import com.wootecam.festivals.domain.organization.entity.Organization;

public record OrganizationResponse(Long organizationId,
String name,
String detail,
String profileImg) {

public static OrganizationResponse from(Organization organization) {
return new OrganizationResponse(organization.getId(),
organization.getName(),
organization.getDetail(),
organization.getProfileImg());
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 리뷰를 바탕으로 다음과 같은 개선 사항을 제안합니다.

  1. 코드 가독성 및 유지 보수성

    • OrganizationResponse 클래스가 더 많은 필드를 포함하게 될 경우, 현재의 생성자와 메서드 구조는 복잡해질 수 있습니다. 이를 위해 Lombok 라이브러리를 사용하여 어노테이션 기반으로 코드를 간소화할 수 있습니다.
    • 각 필드에 대한 설명을 추가하면 코드 이해를 돕는데 유용합니다. Javadoc 주석을 사용하는 것을 권장합니다.
  2. 성능 최적화

    • 현재의 from 메서드는 단순히 getter 메서드를 호출하는 패턴입니다. 데이터가 많아질 경우 성능에 영향을 미칠 수 있습니다. 필요한 데이터만 선택적으로 가져오도록 최적화할 수 있습니다.
  3. 보안 및 데이터 검증

    • Organization에서 반환하는 값들이 null일 가능성이 있으므로, 각 속성에 대해 null 체크를 하여 안전하게 처리하는 것이 좋습니다. 예를 들어, Optional을 사용하여 null 값을 피하거나, 기본값 지정 로직을 구현할 수 있습니다.
    • profileImg 필드에 대한 유효성을 검사하는 논리를 추가하여 외부 데이터가 악의적인 내용을 포함하지 않도록 강화해야 합니다.

이러한 개선 사항을 적용하면 코드의 가독성, 성능, 보안을 크게 향상시킬 수 있습니다.

Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import com.wootecam.festivals.global.audit.BaseEntity;
import jakarta.persistence.Column;
import jakarta.persistence.Entity;
import jakarta.persistence.EnumType;
import jakarta.persistence.Enumerated;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.GenerationType;
import jakarta.persistence.Id;
Expand All @@ -24,6 +26,7 @@ public class OrganizationMember extends BaseEntity {
private Long organizationId;
@Column(nullable = false)
private Long memberId;
@Enumerated(EnumType.STRING)
private OrganizationRole role;

@Builder

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 변경의 리뷰를 다음 세 가지 측면에서 진행하겠습니다:

1. 코드 가독성 및 유지보수성

  • 주석 추가: 클래스와 주요 메서드에 대한 주석을 추가하여 코드의 목적과 기능을 설명하세요. 이는 코드의 이해를 돕고, 향후 수정 시 유용합니다.
  • 일관성 있는 네이밍: organizationId, memberId 등 변수명이 직관적이지만, 더 구체적으로 표현할 수 있다면 좋습니다. 예를 들어, orgId, memberUniqueId처럼 명확히 할 수 있습니다.

2. 성능 최적화

  • Lazy Loading 고려: @OneToMany 또는 @ManyToOne 관계가 있을 경우, 데이터 페치를 최적화하기 위해 Lazy Loading을 사용하는 것이 좋습니다. 불필요한 데이터로드를 방지할 수 있습니다.

3. 보안 및 데이터 유효성 검사

  • 유효성 검사 추가: memberId와 같은 필드에 대해 ID 형식 검증이나 존재 여부 확인(예: null 체크)을 추가하세요. 이를 통해 SQL 인젝션이나 잘못된 데이터 입력을 방지할 수 있습니다.
  • 역할(enum) 제한: OrganizationRole enum을 사용할 때 적절한 역할만 포함되도록 평가는 추가하는 것을 권장합니다. 예를 들어, 새로운 역할을 추가할 경우 알림을 추가하거나 로그를 남기는 방식으로 관리 가능합니다.

이러한 개선 사항들을 적용하면 코드의 품질을 높이고 나중에 발생할 수 있는 문제를 예방할 수 있습니다.

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package com.wootecam.festivals.domain.organization.exception;

import com.wootecam.festivals.global.docs.EnumType;
import com.wootecam.festivals.global.exception.ErrorCode;
import lombok.Getter;
import org.springframework.http.HttpStatus;

@Getter
public enum OrganizationErrorCode implements ErrorCode, EnumType {

ORGANIZATION_NOT_FOUND(HttpStatus.NOT_FOUND, "ORG-0001", "해당하는 조직을 찾을 수 없습니다."),
;

private final HttpStatus httpStatus;
private final String code;
private final String message;

OrganizationErrorCode(HttpStatus httpStatus, String code, String message) {
this.httpStatus = httpStatus;
this.code = code;
this.message = message;
}

@Override
public String getName() {
return name();
}

@Override
public String getDescription() {
return httpStatus + ": " + code + " - " + message;
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. 코드 가독성 및 유지보수성:

    • 주석 추가: 각 필드와 메서드에 대한 설명을 포함하는 주석을 추가하여 이해를 돕습니다.
    • 코드 포맷팅: 열거형 상수를 한 줄에 하나씩 배치하여 나중에 추가할 때 쉽게 볼 수 있도록 합니다.
  2. 성능 최적화:

    • Enum 상수를 미리 로드하므로 성능 문제는 크지 않지만, 필요시 빌더 패턴이나 팩토리 메서드를 고려하여 코드의 유연성을 높일 수 있습니다.
  3. 보안 및 데이터 검증:

    • 메시지 필드에서 사용자 입력을 받을 경우 SQL 인젝션이나 XSS 공격을 방지하기 위해 입력값 검증 로직을 강화합니다.
    • HttpStatus 코드는 이미 안전한 정적 상수가므로 별도의 추가 조치는 필요하지 않습니다.

주요 개선 사항 요약

  • 주석 추가: 각 변수와 메서드에 대한 설명을 작성.
  • Enum 상수 관리 방안을 마련 (예: Builder 패턴 등).
  • 사용자 입력 검증 로직 강화하여 보안을 높임.

Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
package com.wootecam.festivals.domain.organization.repository;

import com.wootecam.festivals.domain.organization.entity.Organization;
import java.util.Optional;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Query;

public interface OrganizationRepository extends JpaRepository<Organization, Long> {

@Query("select o from Organization o where o.id = :organizationId and o.isDeleted = false")
Optional<Organization> findById(Long organizationId);
Comment on lines +10 to +11
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

현종님한테 전해들었던 이야기이긴한데 저는 단건 조회인 경우에는 isDeleted 상관없이 무조건 조회하고, 여러개 조회인 경우에 isDeleted=false 로 조건 걸었던거 같아요.

어차피 서비스에 단건 조회 요청 들어오는건 어차피 여러개 조회할때 isDeleted=false 로 한번 필터링된 것이기 때문에요!

}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. 코드 가독성과 유지보수성:

    • findById 메소드에 대한 설명을 추가하는 Javadoc 주석을 달아 코드의 목적을 명확히 하면 좋습니다.
    • 메소드 이름을 findActiveById로 변경하여, 검색 조건이 포함된 의미를 더 분명하게 표현해주세요.
  2. 성능 최적화:

    • 이 쿼리는 이미 특정 필드를 조건으로 사용하고 있어 성능적으로 적절합니다. 다만, 인덱스가 필요할 수 있으므로 데이터베이스에서 isDeleted 필드에 인덱스를 추가하는 것을 고려하세요.
  3. 보안 및 데이터 검증:

    • 현재 수정된 부분은 안전하지만, 입력받는 organizationId 값이 유효한지 여부를 검증하는 로직(예: null 체크)을 추가하는 것이 좋습니다. 이를 통해 잘못된 데이터가 전달되는 것을 방지할 수 있습니다.
    • @Query 어노테이션을 사용할 때, SQL 인젝션 공격에 대비해 파라미터 바인딩이 잘 적용되고 있는지 확인하세요.

결론적으로, 주석 추가와 메소드명 변경으로 가독성을 높이고, 데이터 검증을 통해 보안을 강화하세요.

Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package com.wootecam.festivals.domain.organization.service;

import com.wootecam.festivals.domain.organization.dto.OrganizationCreateDto;
import com.wootecam.festivals.domain.organization.dto.OrganizationResponse;
import com.wootecam.festivals.domain.organization.entity.Organization;
import com.wootecam.festivals.domain.organization.entity.OrganizationMember;
import com.wootecam.festivals.domain.organization.entity.OrganizationRole;
import com.wootecam.festivals.domain.organization.exception.OrganizationErrorCode;
import com.wootecam.festivals.domain.organization.repository.OrganizationMemberRepository;
import com.wootecam.festivals.domain.organization.repository.OrganizationRepository;
import com.wootecam.festivals.global.exception.type.ApiException;
import com.wootecam.festivals.global.utils.AuthenticationUtils;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
Expand All @@ -27,6 +30,13 @@ public Long createOrganization(OrganizationCreateDto organizationCreateDto) {
return newOrganization.getId();
}

public OrganizationResponse findOrganization(Long organizationId) {
Organization organization = organizationRepository.findById(organizationId)
.orElseThrow(() -> new ApiException(OrganizationErrorCode.ORGANIZATION_NOT_FOUND));

return OrganizationResponse.from(organization);
}

private Organization saveOrganization(OrganizationCreateDto organizationCreateDto) {
return organizationRepository.save(organizationCreateDto.toEntity());
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 리뷰 요약:

  1. 코드 가독성과 유지보수성:

    • 메서드 이름과 DTO를 명확하게 사용하여 코드의 의미를 부여하세요. 예를 들어, saveOrganization 메서드에서 DTO가 무엇인지 더 직관적으로 알 수 있게 설명하는 주석을 추가하면 좋습니다.
    • 공통적으로 사용되는 로직은 헬퍼 메서드로 분리하여 중복을 줄이고 가독성을 높이세요.
  2. 성능 최적화:

    • findById는 내부적으로 Optional을 처리하므로, 만약 다수의 ID를 여러 번 조회해야 한다면, 배치 쿼리를 검토해 볼 필요가 있습니다.
    • 레포지토리에서 필요한 필드만 선택적으로 조회하는 방법(@Query)을 도입하면 성능상 이점을 얻을 수 있습니다.
  3. 보안 및 데이터 검증:

    • 데이터베이스에 저장하기 전, OrganizationCreateDto의 유효성을 검증하는 로직을 추가하여 잘못된 데이터 저장을 방지하세요.
    • 더욱 안전한 API를 위해, 민감한 정보에 대한 접근 제어(예: 인증 및 권한 체크) 메커니즘을 추가할 것을 권장합니다.

이러한 사항을 고려하면 코드의 전체적인 품질과 안정성을 향상시킬 수 있습니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Java 백엔드 코드 변경사항에 대한 리뷰를 다음과 같이 정리하였습니다.

  1. 코드 가독성 및 유지보수성:

    • findOrganization 메서드는 Optional을 사용하여 null 처리를 명확하게 하고 있어 좋습니다.
    • 오류 메시지를 OrganizationErrorCode로 관리하는 점이 일관성 있고 유지보수에 유리합니다.
    • DTO 변환 메서드(from)의 이름이 직관적입니다. 그러나 추가적인 주석이나 문서화를 통해 메서드의 동작을 설명하면 더 좋습니다.
  2. 성능 최적화:

    • 현재 조회 방식에서 성능 개선을 위한 캐싱 전략 도입을 고려해 볼 수 있습니다. 자주 조회되는 조직 정보에 대해 캐시를 사용하면 DB 호출을 줄일 수 있습니다.
  3. 보안 및 데이터 검증:

    • findOrganization 메서드 내에서 organizationId에 대한 추가적인 유효성 검사가 필요할 수 있습니다. 예를 들어, 음수값이 입력될 경우 적절한 예외 처리를 하는 것이 좋습니다.
    • API 요청을 처리하기 전, 인증 및 권한 확인 로직이 있는지 여부를 점검해야 합니다. 특히 민감한 데이터에 접근하는 경우에는 더욱 중요합니다.

이와 같은 개선점을 반영하면 코드의 품질과 보안성을 높이는 데 기여할 수 있습니다.

Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,20 @@
package com.wootecam.festivals.domain.organization.controller;

import static org.mockito.BDDMockito.given;
import static org.springframework.restdocs.payload.PayloadDocumentation.beneathPath;
import static org.springframework.restdocs.payload.PayloadDocumentation.fieldWithPath;
import static org.springframework.restdocs.payload.PayloadDocumentation.requestFields;
import static org.springframework.restdocs.payload.PayloadDocumentation.responseFields;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

import com.wootecam.festivals.docs.utils.RestDocsSupport;
import com.wootecam.festivals.domain.organization.dto.OrganizationCreateDto;
import com.wootecam.festivals.domain.organization.dto.OrganizationResponse;
import com.wootecam.festivals.domain.organization.exception.OrganizationErrorCode;
import com.wootecam.festivals.domain.organization.service.OrganizationService;
import com.wootecam.festivals.global.exception.type.ApiException;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
Expand All @@ -35,7 +40,7 @@ protected Object initController() {
}

@Test
@DisplayName("Organization을 생성한다")
@DisplayName("organization을 생성한다")
void create() throws Exception {
this.mockMvc.perform(post("/api/v1/organizations")
.content(objectMapper.writeValueAsString(
Expand All @@ -57,4 +62,37 @@ void create() throws Exception {
fieldWithPath("organizationId").type(JsonFieldType.NUMBER).description("생성된 조직 ID")
)));
}

@Test
@DisplayName("organization을 조회한다")
void success_find() throws Exception {
given(organizationService.findOrganization(1L))
.willReturn(new OrganizationResponse(1L, "testOrganization", "test detail", "test profile"));

this.mockMvc.perform(get("/api/v1/organizations/1"))
.andExpect(status().isOk())
.andDo(restDocs.document(
responseFields(
beneathPath("data").withSubsectionId("data"),
fieldWithPath("organizationId").type(JsonFieldType.NUMBER).description("조회한 조직 ID"),
fieldWithPath("name").type(JsonFieldType.STRING).description("조회한 조직 이름"),
fieldWithPath("detail").type(JsonFieldType.STRING).description("조회한 조직 설명"),
fieldWithPath("profileImg").type(JsonFieldType.STRING).description("조회한 조직 프로필 이미지")
)));
}

@Test
@DisplayName("존재하지 않는 organization을 조회하면 404 에러를 반환한다")
void fail_find() throws Exception {
given(organizationService.findOrganization(1L))
.willThrow(new ApiException(OrganizationErrorCode.ORGANIZATION_NOT_FOUND));

this.mockMvc.perform(get("/api/v1/organizations/1"))
.andExpect(status().isNotFound())
.andDo(restDocs.document(
responseFields(
fieldWithPath("errorCode").type(JsonFieldType.STRING).description("에러 코드"),
fieldWithPath("message").type(JsonFieldType.STRING).description("에러 설명 메시지")
)));
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 스니펫을 검토한 결과, 다음과 같은 개선 사항을 제안합니다.

  1. 코드 가독성 및 유지보수성:

    • 테스트 함수의 주석이나 설명을 추가하여 각 테스트 목적을 명확히 할 것.
    • @DisplayName을 영어로 작성하는 것이 좋습니다. 여러 개발자가 있는 환경에서는 코드 이해를 더 쉽게 합니다.
  2. 성능 최적화:

    • 테스트에서 만든 더미 데이터(OrganizationResponse)를 빌드할 때, 고정된 값을 사용할 경우 별도의 팩토리 클래스를 도입하여 테스트 데이터 생성을 일관되게 관리할 수 있습니다.
  3. 보안 및 데이터 검증:

    • findOrganization 메소드에 대한 예외 처리를 통해 적절한 HTTP 상태 코드를 반환하도록 하였지만, 잘못된 경로 접근에 대한 보안 검증도 고려해야 합니다. 예를 들어, 파라미터 값이 유효한지 체크하여 SQL 인젝션 등을 방지하는 로직이 필요합니다.

이러한 개선 사항들을 반영하면 코드의 전체적인 품질을 높일 수 있습니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드 변경 내역에 대한 리뷰를 다음과 같이 요약합니다.

1. 코드 가독성 및 유지보수성

  • 기술 이름 통일성: 코드의 여러 부분에서 Organization을 소문자로 쓴 'organization'으로 변경했는데, 이는 일관성을 높이고 가독성을 개선했습니다.
  • 메서드와 변수 이름: 테스트 메서드의 이름은 명확하고 설명적입니다. 그러나 ‘success_find()’와 ‘fail_find()’는 더 구체적인 이름으로 변경하는 것이 좋습니다 (예: shouldReturnOrganizationWhenExists()).
  • 주석 추가: 주요 로직이나 결정적인 부분에 대한 주석을 추가하여 코드 이해를 돕는 것도 좋습니다.

2. 성능 최적화

  • MockMvc 설정: MockMvc 설정 시 필요 없는 데이터 초기화를 피하고 단순화하여 테스트 속도를 개선할 수 있는 방법을 고려해보세요.
  • DTO 활용: 필요하지 않은 모든 필드를 포함하는 DTO 객체를 반환하는 대신, 필요한 필드만 반환하도록 DTO를 단순화하면 메모리 사용을 줄일 수 있습니다.

3. 보안 및 데이터 검증

  • 입력 검증: findOrganization(1L)와 같은 호출에서 ID 값 검증 로직을 추가하여 유효하지 않은 요청에 대해 더 엄격한 검사를 수행하는 것이 좋습니다.
  • API 예외 처리: ApiException를 사용할 때 에러 처리 로직을 집중화하여, 예외가 발생했을 때 적절한 응답 메시지를 제공하도록 처리하는 것이 중요합니다.

제안된 개선 사항을 통해 코드 품질을 높이고 유지보수를 용이하게 만들 수 있습니다.

Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package com.wootecam.festivals.domain.organization.service;

import static com.wootecam.festivals.utils.Fixture.createOrganization;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertAll;

import com.wootecam.festivals.domain.member.repository.MemberRepository;
import com.wootecam.festivals.domain.organization.dto.OrganizationCreateDto;
import com.wootecam.festivals.domain.organization.dto.OrganizationResponse;
import com.wootecam.festivals.domain.organization.entity.Organization;
import com.wootecam.festivals.domain.organization.entity.OrganizationMember;
import com.wootecam.festivals.domain.organization.entity.OrganizationRole;
import com.wootecam.festivals.domain.organization.repository.OrganizationMemberRepository;
Expand Down Expand Up @@ -47,6 +50,7 @@ void setUp() {
@Nested
@DisplayName("로그인한 사용자가 Organization 생성 시")
class create_organization {

Authentication authentication = new Authentication(1L, "testUser", "testEmail");

@BeforeEach
Expand Down Expand Up @@ -81,4 +85,36 @@ void it_returns_new_organization() {
}
}
}

@Nested
@DisplayName("Organization 조회 시")
class find_organization {

Organization organization;
Long organizationId;

@BeforeEach
void setUp() {
organization = createOrganization("validName", "profile.jpg", "This is a valid detail.");
organizationId = organizationRepository.save(organization).getId();
}

@Nested
@DisplayName("Organization id가 주어지면")
class when_id_is_given {

@Test
@DisplayName("해당 organization 정보를 조회하여 반환한다")
void it_returns_organization() {
OrganizationResponse organizationResponse = organizationService.findOrganization(organizationId);

assertAll(() -> {
assertThat(organizationResponse.organizationId()).isEqualTo(organizationId);
assertThat(organizationResponse.name()).isEqualTo(organization.getName());
assertThat(organizationResponse.detail()).isEqualTo(organization.getDetail());
assertThat(organizationResponse.profileImg()).isEqualTo(organization.getProfileImg());
});
}
}
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 리뷰 요약:

1. 코드 가독성 및 유지보수성

  • 패키지와 클래스 이름: 패키지 및 클래스 이름은 명확하지만, 테스트 클래스의 이름을 좀 더 직관적으로 변경하는 것이 좋습니다. 예를 들어, create_organization 대신 CreateOrganizationTests와 같은 형태가 더 이해하기 쉽습니다.

  • 주석 추가: 각 테스트 케이스 앞에 설명 주석을 추가하여 해당 테스트가 어떤 기능을 검증하고 있는지 쉽게 파악할 수 있도록 해주세요.

2. 성능 최적화

  • 중복 코드 제거: setUp() 메서드에서 조직을 반복적으로 생성하는데, 필요할 때만 생성을 하도록 수정하면 불필요한 객체 생성을 줄일 수 있습니다. 특히, ID를 얻는 부분도 한 번만 실행되도록 조정할 수 있습니다.

3. 보안 및 데이터 검증

  • 입력값 검증: createOrganization 메서드를 호출하는 부분에서 입력값이 유효한지 검증하는 로직을 추가하세요. 예를 들어, 조직 이름이 비어 있거나 너무 긴 경우를 체크해야 합니다.

  • 예외 처리: findOrganization 메서드에서 존재하지 않는 ID를 요청했을 때 적절한 예외 처리를 할 수 있는지를 확인합니다. 빈 값을 반환하거나 일반적인 오류 메시지가 아닌 구체적인 오류를 던지는 것이 좋습니다.

추가 제안

  • 사용되는 DTO와 엔티티 간의 변환 과정이 없다면, 필요한 변환 로직을 추가하여 클린 아키텍처를 유지하세요.
  • 반복적인 테스트 코드에는 공통적인 설정 로직을 사용하여 DRY(Don't Repeat Yourself) 원칙을 적용합니다.

이러한 개선 사항을 통해 코드의 가독성과 안전성을 높이고 성능을 최적화할 수 있습니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

다음은 제공된 Java 백엔드 코드의 변경 사항에 대한 평가와 개선 제안입니다:

1. 코드 가독성과 유지보수성

  • 코드 정리: import 문을 알파벳 순서로 정렬하여 가독성을 높이고 다른 개발자가 쉽게 이해할 수 있도록 합니다.
  • 주석 추가: 각 주요 블록이나 메소드에 주석을 추가해 그 기능과 목적을 명확히 하는 것이 좋습니다.
  • 네이밍 일관성: 클래스 및 메소드 이름이 명확하고 일관되도록 하여 코드를 읽는 사람이 역할을 쉽게 파악할 수 있게합니다.

2. 성능 최적화

  • 불필요한 객체 생성을 피하기: setUp 메소드에서 매번 새로운 Organization 객체를 만드는 대신, 필요할 때만 생성하도록 수정하면 메모리 사용량을 줄일 수 있습니다.
  • 데이터베이스 조회 최적화: 만약 organizationRepository.save()가 중복 저장 가능성이 있다면 중복 데이터를 방지하는 로직을 고려해야 합니다.

3. 보안 및 데이터 검증

  • 입력 유효성 검사: createOrganization 호출 시 입력 값(validName, profile.jpg, detail)의 유효성을 점검하여 SQL 인젝션이나 악의적인 데이터 입력을 방지하도록 합니다.
  • 인증 및 권한 검증: Organization 조회 시, 사용자가 해당 조직에 대한 접근 권한을 가지고 있는지 확인하는 로직을 넣는 것이 중요합니다.

이 조치들은 코드의 전반적인 품질을 높이고, 장기적으로 유지보수가 용이하도록 할 것입니다.

17 changes: 17 additions & 0 deletions src/test/java/com/wootecam/festivals/utils/Fixture.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package com.wootecam.festivals.utils;

import com.wootecam.festivals.domain.organization.entity.Organization;

public final class Fixture {

private Fixture() {
}

public static Organization createOrganization(String name, String detail, String profileImg) {
return Organization.builder()
.name(name)
.detail(detail)
.profileImg(profileImg)
.build();
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 리뷰 결과는 다음과 같습니다:

  1. 가독성과 유지보수성:

    • 코드의 읽기가 쉬운 편입니다. 하지만 각 메소드에 대한 Javadoc 주석을 추가하면 더욱 명확하게 이해할 수 있습니다.
    • 예를 들어, createOrganization 메소드의 기능을 설명하는 주석이 있으면 좋습니다.
  2. 성능 최적화:

    • 현재 코드는 성능상으로 문제는 보이지 않지만, Organization.builder() 패턴이 사용되므로 빌더 패턴이 필요하지 않은 경우, 직접 생성자를 사용하는 것이 성능 측면에서 더 나을 수 있습니다. 이 부분은 실제 사용 상황에 따라 고려해야 합니다.
  3. 보안 및 데이터 검증:

    • 전달받는 파라미터(name, detail, profileImg)에 대한 유효성을 검사하는 로직이 없습니다. Null 또는 공백 문자열에 대한 검증을 추가하여 잘못된 데이터가 Organization 객체로 변환되지 않도록 해야 합니다.
    • 예를 들어:
      if (name == null || name.trim().isEmpty()) {
          throw new IllegalArgumentException("Name must not be empty");
      }

제안 요약:

  • Javadoc 주석 추가
  • 필요에 따라 빌더 패턴 사용 검토
  • 입력 파라미터 검증 로직 추가

이러한 개선 사항들은 코드의 전반적인 품질을 높이고 유지보수를 용이하게 할 것입니다.

Loading