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.

Sign up for GitHub

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

[DDING-55] Feed 삭제 API 구현 #200

Merged
merged 8 commits into from
Jan 8, 2025
Merged

[DDING-55] Feed 삭제 API 구현 #200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a26f1de
feat : 피드 삭제 API 구현
KoSeonJe Jan 8, 2025
f845b05
test : 피드 삭제 API 테스트 작성
KoSeonJe Jan 8, 2025
f893e77
feat : @Transactional 어노테이션 추가
KoSeonJe Jan 8, 2025
3846a83
test : 테스트 에러 수정
KoSeonJe Jan 8, 2025
34ef7ed
test : video일 경우 테스트 추가
KoSeonJe Jan 8, 2025
c982749
refactor : fileMetaData의 FileStatus Delete 변환 로직 수정
KoSeonJe Jan 8, 2025
639d68a
fix : @Transactional 추가
KoSeonJe Jan 8, 2025
eb38e0a
refactor : FeedType에 따른 FileMetaData 삭제로직 수정
KoSeonJe Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions src/main/java/ddingdong/ddingdongBE/domain/feed/api/ClubFeedApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import jakarta.validation.Valid;
import org.springframework.http.HttpStatus;
import org.springframework.security.core.annotation.AuthenticationPrincipal;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.PutMapping;
Expand All @@ -18,14 +19,14 @@
import org.springframework.web.bind.annotation.ResponseStatus;

@Tag(name = "Feed - Club", description = "Feed API")
@RequestMapping("/server")
@RequestMapping("/server/central")
public interface ClubFeedApi {

@Operation(summary = "동아리 피드 생성 API")
@ApiResponse(responseCode = "201", description = "동아리 피드 생성 성공")
@ResponseStatus(HttpStatus.CREATED)
@SecurityRequirement(name = "AccessToken")
@PostMapping("/central/clubs/feeds")
@PostMapping("/clubs/feeds")
void createFeed(
@RequestBody @Valid CreateFeedRequest createFeedRequest,
@AuthenticationPrincipal PrincipalDetails principalDetails
Expand All @@ -35,9 +36,19 @@ void createFeed(
@ApiResponse(responseCode = "204", description = "동아리 피드 수정 성공")
@ResponseStatus(HttpStatus.NO_CONTENT)
@SecurityRequirement(name = "AccessToken")
@PutMapping("/central/clubs/feeds/{feedId}")
@PutMapping("/clubs/feeds/{feedId}")
void updateFeed(
@PathVariable("feedId") Long feedId,
@RequestBody @Valid UpdateFeedRequest updateFeedRequest
);

@Operation(summary = "동아리 피드 삭제 API")
@ApiResponse(responseCode = "204", description = "동아리 피드 삭제 성공")
@ResponseStatus(HttpStatus.NO_CONTENT)
@SecurityRequirement(name = "AccessToken")
@DeleteMapping("/clubs/feeds/{feedId}")
void deleteFeed(
@PathVariable("feedId") Long feedId
);

}
5 changes: 5 additions & 0 deletions src/main/java/ddingdong/ddingdongBE/domain/feed/controller/ClubFeedController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,9 @@ public void updateFeed(
) {
facadeClubFeedService.update(updateFeedRequest.toCommand(feedId));
}

@Override
public void deleteFeed(Long feedId) {
facadeClubFeedService.delete(feedId);
}
Comment on lines +35 to +38
Copy link

@coderabbitai coderabbitai bot Jan 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

보안 검증이 누락된 것이 확인되었습니다.

ClubFeedApi 인터페이스에 정의된 보안 요구사항(@securityrequirement)에 맞게 구현되어야 하나, updateFeed와 deleteFeed 메서드 모두에서 인증 파라미터(PrincipalDetails)가 누락되어 있습니다. 다음과 같이 수정이 필요합니다:

-    public void updateFeed(Long feedId, UpdateFeedRequest updateFeedRequest) {
+    public void updateFeed(Long feedId, UpdateFeedRequest updateFeedRequest, PrincipalDetails principalDetails) {
+        User user = principalDetails.getUser();
         facadeClubFeedService.update(updateFeedRequest.toCommand(feedId));
     }

-    public void deleteFeed(Long feedId) {
+    public void deleteFeed(Long feedId, PrincipalDetails principalDetails) {
+        User user = principalDetails.getUser();
         facadeClubFeedService.delete(feedId);
     }
🔗 Analysis chain

보안 검증이 필요합니다.

다른 엔드포인트들과 달리 deleteFeed 메서드에는 인증 검사(PrincipalDetails)가 누락되어 있습니다. 피드 삭제 권한이 있는 사용자만 접근할 수 있도록 보안 검증이 필요합니다.

다음과 같이 수정을 제안합니다:

-    public void deleteFeed(Long feedId) {
+    public void deleteFeed(Long feedId, PrincipalDetails principalDetails) {
+        User user = principalDetails.getUser();
         facadeClubFeedService.delete(feedId);
     }
🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found

Script:

#!/bin/bash
# Get the complete controller file
cat src/main/java/ddingdong/ddingdongBE/domain/feed/controller/ClubFeedController.java

# Search for security related configurations
rg "@Security|@PreAuthorize|@Secured" -A 2

# Find service implementation
fd "ClubFeedService" --type f

Length of output: 20533

Copy link

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

}
10 changes: 9 additions & 1 deletion src/main/java/ddingdong/ddingdongBE/domain/feed/entity/FeedType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,17 @@
package ddingdong.ddingdongBE.domain.feed.entity;

import ddingdong.ddingdongBE.domain.filemetadata.entity.DomainType;
import java.util.Arrays;
import lombok.Getter;
import lombok.RequiredArgsConstructor;

@Getter
@RequiredArgsConstructor
public enum FeedType {
IMAGE, VIDEO;
IMAGE(DomainType.FEED_IMAGE),
VIDEO(DomainType.FEED_VIDEO);

private final DomainType domainType;

public static FeedType findByContentType(String contentType) {
return Arrays.stream(values())
Expand Down
1 change: 1 addition & 0 deletions src/main/java/ddingdong/ddingdongBE/domain/feed/service/FacadeClubFeedService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ public interface FacadeClubFeedService {

void update(UpdateFeedCommand command);

void delete(Long feedId);
}
9 changes: 9 additions & 0 deletions src/main/java/ddingdong/ddingdongBE/domain/feed/service/FacadeClubFeedServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ public void create(CreateFeedCommand command) {

if (feed.isImage()) {
fileMetaDataService.updateStatusToCoupled(command.mediaId(), DomainType.FEED_IMAGE, createdId);
return;
}

if (feed.isVideo()) {
Expand All @@ -43,4 +44,12 @@ public void update(UpdateFeedCommand command) {
Feed updateFeed = command.toEntity();
originFeed.update(updateFeed);
}

@Override
@Transactional
public void delete(Long feedId) {
Feed feed = feedService.getById(feedId);
feedService.delete(feed);
fileMetaDataService.updateStatusToDelete(feed.getFeedType().getDomainType(), feed.getId());
}
Comment on lines +50 to +54
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

권한 검증 로직 추가 필요

현재 구현에서는 피드 삭제 전에 사용자의 권한이나 소유권을 확인하는 로직이 없습니다. 보안을 위해 삭제 권한 검증을 추가하는 것이 좋겠습니다.

예시 구현:

 @Transactional
 public void delete(Long feedId) {
     Feed feed = feedService.getById(feedId);
+    Club club = clubService.getByUserId(SecurityUtil.getCurrentUserId());
+    if (!feed.getClub().equals(club)) {
+        throw new UnauthorizedException("피드 삭제 권한이 없습니다.");
+    }
     feedService.delete(feed);
     fileMetaDataService.updateStatusToDelete(feed.getFeedType().getDomainType(), feed.getId());
 }

Committable suggestion skipped: line range outside the PR's diff.

}
2 changes: 2 additions & 0 deletions src/main/java/ddingdong/ddingdongBE/domain/feed/service/FeedService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,6 @@ public interface FeedService {
Feed getById(Long feedId);

Long create(Feed feed);

void delete(Feed feed);
}
6 changes: 6 additions & 0 deletions src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,10 @@ public Long create(Feed feed) {
Feed savedFeed = feedRepository.save(feed);
return savedFeed.getId();
}

@Override
@Transactional
public void delete(Feed feed) {
feedRepository.delete(feed);
}
}
7 changes: 6 additions & 1 deletion src/main/java/ddingdong/ddingdongBE/domain/filemetadata/entity/DomainType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
package ddingdong.ddingdongBE.domain.filemetadata.entity;

import lombok.Getter;
import lombok.RequiredArgsConstructor;

@RequiredArgsConstructor
@Getter
public enum DomainType {
CLUB_PROFILE,
CLUB_INTRODUCTION,
Expand All @@ -11,5 +16,5 @@ public enum DomainType {
BANNER_WEB_IMAGE,
BANNER_MOBILE_IMAGE,
FEED_IMAGE,
FEED_VIDEO
FEED_VIDEO;
}
1 change: 1 addition & 0 deletions src/main/java/ddingdong/ddingdongBE/domain/filemetadata/service/FileMetaDataServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ public void updateStatusToCoupled(String id, DomainType domainType, Long entityI
fileMetaData.updateStatus(COUPLED);
}

@Transactional
@Override
public void updateStatusToCoupledWithOrder(
List<FileMetaDataIdOrderDto> fileMetaDataIdOrderDtos,
Expand Down
71 changes: 71 additions & 0 deletions src/test/java/ddingdong/ddingdongBE/domain/feed/service/FacadeClubFeedServiceImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import ddingdong.ddingdongBE.domain.filemetadata.entity.FileMetaData;
import ddingdong.ddingdongBE.domain.filemetadata.entity.FileStatus;
import ddingdong.ddingdongBE.domain.filemetadata.repository.FileMetaDataRepository;
import ddingdong.ddingdongBE.domain.filemetadata.service.FileMetaDataServiceImpl;
import ddingdong.ddingdongBE.domain.scorehistory.entity.Score;
import ddingdong.ddingdongBE.domain.user.entity.User;
import ddingdong.ddingdongBE.domain.user.repository.UserRepository;
Expand Down Expand Up @@ -47,6 +48,8 @@ class FacadeClubFeedServiceImplTest extends TestContainerSupport {
private EntityManager entityManager;

private final FixtureMonkey fixtureMonkey = FixtureMonkeyFactory.getNotNullBuilderIntrospectorMonkey();
@Autowired
private FileMetaDataServiceImpl fileMetaDataServiceImpl;

@DisplayName("요청된 Command를 사용하여 feed를 생성하며, FileMetaData를 Couple 상태로 변경한다.")
@Test
Expand Down Expand Up @@ -111,4 +114,72 @@ void update() {
assertThat(finded).isNotNull();
assertThat(finded.getActivityContent()).isEqualTo("변경된 활동내용");
}

@DisplayName("주어진 feedId를 가진 Feed 엔터티를 삭제 및 fileMetaData 상태를 DELETED로 변경 - IMAGE")
@Test
void deleteImage() {
// given
UUID uuid = UuidCreator.getTimeOrderedEpoch();


Feed savedFeed = feedRepository.save(
fixtureMonkey.giveMeBuilder(Feed.class)
.set("feedType", FeedType.IMAGE)
.set("activityContent", "활동내용")
.set("club", null)
.sample()
);
fileMetaDataRepository.save(
fixtureMonkey.giveMeBuilder(FileMetaData.class)
.set("id", uuid)
.set("entityId", savedFeed.getId())
.set("domainType", DomainType.FEED_IMAGE)
.set("fileStatus", FileStatus.COUPLED)
.sample()
);
entityManager.flush();
// when
facadeClubFeedService.delete(savedFeed.getId());
entityManager.flush();
// then
Feed feed = feedRepository.findById(savedFeed.getId()).orElse(null);
FileMetaData fileMetaData = fileMetaDataRepository.findById(uuid).orElse(null);
assertThat(feed).isNull();
assertThat(fileMetaData).isNotNull();
assertThat(fileMetaData.getFileStatus()).isEqualTo(FileStatus.DELETED);
}

@DisplayName("주어진 feedId를 가진 Feed 엔터티를 삭제 및 fileMetaData 상태를 DELETED로 변경 - VIDEO")
@Test
void deleteVideo() {
// given
UUID uuid = UuidCreator.getTimeOrderedEpoch();


Feed savedFeed = feedRepository.save(
fixtureMonkey.giveMeBuilder(Feed.class)
.set("feedType", FeedType.VIDEO)
.set("activityContent", "활동내용")
.set("club", null)
.sample()
);
fileMetaDataRepository.save(
fixtureMonkey.giveMeBuilder(FileMetaData.class)
.set("id", uuid)
.set("entityId", savedFeed.getId())
.set("domainType", DomainType.FEED_VIDEO)
.set("fileStatus", FileStatus.COUPLED)
.sample()
);
entityManager.flush();
// when
facadeClubFeedService.delete(savedFeed.getId());
entityManager.flush();
// then
Feed feed = feedRepository.findById(savedFeed.getId()).orElse(null);
FileMetaData fileMetaData = fileMetaDataRepository.findById(uuid).orElse(null);
assertThat(feed).isNull();
assertThat(fileMetaData).isNotNull();
assertThat(fileMetaData.getFileStatus()).isEqualTo(FileStatus.DELETED);
}
Comment on lines +152 to +184
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

비디오 Feed 삭제 테스트의 중복 코드를 제거해야 합니다.

deleteVideo() 테스트 메서드가 deleteImage() 메서드와 거의 동일한 구조를 가지고 있습니다. 테스트 코드의 중복을 제거하고 가독성을 높이기 위해 파라미터화된 테스트로 리팩토링하는 것이 좋을 것 같습니다.

다음과 같이 개선해 보시는 것은 어떨까요?:

@ParameterizedTest
@EnumSource(FeedType.class)
void deleteFeed_ShouldDeleteFeedAndUpdateFileMetaData(FeedType feedType) {
    // given
    UUID uuid = UuidCreator.getTimeOrderedEpoch();
    Feed savedFeed = createFeed(feedType);
    createFileMetaData(uuid, savedFeed, 
        feedType == FeedType.IMAGE ? DomainType.FEED_IMAGE : DomainType.FEED_VIDEO);
    entityManager.flush();

    // when
    facadeClubFeedService.delete(savedFeed.getId());
    entityManager.flush();

    // then
    assertThat(feedRepository.findById(savedFeed.getId())).isEmpty();
    FileMetaData fileMetaData = fileMetaDataRepository.findById(uuid).orElse(null);
    assertThat(fileMetaData)
        .isNotNull()
        .extracting(FileMetaData::getFileStatus)
        .isEqualTo(FileStatus.DELETED);
}

private Feed createFeed(FeedType feedType) {
    return feedRepository.save(
        fixtureMonkey.giveMeBuilder(Feed.class)
            .set("feedType", feedType)
            .set("activityContent", "활동내용")
            .set("club", null)
            .sample()
    );
}

}
Loading