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

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

Merged
merged 8 commits into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
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
);

}
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

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!

}
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);
}
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,19 @@ 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);
if (feed.isImage()) {
fileMetaDataService.updateStatusToDelete(DomainType.FEED_IMAGE, feedId);
return;
}

if (feed.isVideo()) {
fileMetaDataService.updateStatusToDelete(DomainType.FEED_VIDEO, feedId);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

p3)
두 조건 모두 수행하는 코드가 상태를 DELETED로 바꾸는 것인데 이미지인지 비디오인지 체크하는 이유가 있을까요?
updateStatusToDelete 메서드가 해당 파리미터를 필요로 하는것이라면 상태면 수정하는 메서드를 새로 만드는게 나을거 같습니다!

}
}
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);
}
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);
}
}
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,36 @@ void update() {
assertThat(finded).isNotNull();
assertThat(finded.getActivityContent()).isEqualTo("변경된 활동내용");
}

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

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