Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
7 changes: 7 additions & 0 deletions src/main/java/io/spring/api/CurrentUserApi.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import lombok.AllArgsConstructor;
import org.springframework.http.ResponseEntity;
import org.springframework.security.core.annotation.AuthenticationPrincipal;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PutMapping;
import org.springframework.web.bind.annotation.RequestBody;
Expand Down Expand Up @@ -48,6 +49,12 @@ public ResponseEntity updateProfile(
return ResponseEntity.ok(userResponse(new UserWithToken(userData, token.split(" ")[1])));
}

@DeleteMapping
public ResponseEntity deleteUser(@AuthenticationPrincipal User currentUser) {
userService.removeUser(currentUser);
return ResponseEntity.noContent().build();
}

private Map<String, Object> userResponse(UserWithToken userWithToken) {
return new HashMap<String, Object>() {
{
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/io/spring/application/user/UserService.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ public void updateUser(@Valid UpdateUserCommand command) {
updateUserParam.getImage());
userRepository.save(user);
}

public void removeUser(User user) {
userRepository.remove(user);
}
}

@Constraint(validatedBy = UpdateUserValidator.class)
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/io/spring/core/user/UserRepository.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,6 @@ public interface UserRepository {
Optional<FollowRelation> findRelation(String userId, String targetId);

void removeRelation(FollowRelation followRelation);

void remove(User user);
}
13 changes: 13 additions & 0 deletions src/main/java/io/spring/graphql/UserMutation.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import io.spring.graphql.DgsConstants.MUTATION;
import io.spring.graphql.exception.GraphQLCustomizeExceptionHandler;
import io.spring.graphql.types.CreateUserInput;
import io.spring.graphql.types.DeletionStatus;
import io.spring.graphql.types.UpdateUserInput;
import io.spring.graphql.types.UserPayload;
import io.spring.graphql.types.UserResult;
Expand Down Expand Up @@ -90,4 +91,16 @@ public DataFetcherResult<UserPayload> updateUser(
.localContext(currentUser)
.build();
}

@DgsData(parentType = MUTATION.TYPE_NAME, field = MUTATION.DeleteUser)
public DeletionStatus deleteUser() {
Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
if (authentication instanceof AnonymousAuthenticationToken
|| authentication.getPrincipal() == null) {
return DeletionStatus.newBuilder().success(false).build();
}
io.spring.core.user.User currentUser = (io.spring.core.user.User) authentication.getPrincipal();
userService.removeUser(currentUser);
return DeletionStatus.newBuilder().success(true).build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,8 @@ public interface ArticleFavoriteMapper {
void insert(@Param("articleFavorite") ArticleFavorite articleFavorite);

void delete(@Param("favorite") ArticleFavorite favorite);

void deleteByUserId(@Param("userId") String userId);

void deleteByArticleUserId(@Param("userId") String userId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,8 @@ public interface ArticleMapper {
void update(@Param("article") Article article);

void delete(@Param("id") String id);

void deleteByUserId(@Param("userId") String userId);

void deleteArticleTagsByUserId(@Param("userId") String userId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,8 @@ public interface CommentMapper {
Comment findById(@Param("articleId") String articleId, @Param("id") String id);

void delete(@Param("id") String id);

void deleteByUserId(@Param("userId") String userId);

void deleteByArticleUserId(@Param("userId") String userId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,10 @@ public interface UserMapper {
void saveRelation(@Param("followRelation") FollowRelation followRelation);

void deleteRelation(@Param("followRelation") FollowRelation followRelation);

void delete(@Param("id") String id);

void deleteFollowsByUserId(@Param("userId") String userId);

void deleteFollowsTargetingUser(@Param("userId") String userId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,32 @@
import io.spring.core.user.FollowRelation;
import io.spring.core.user.User;
import io.spring.core.user.UserRepository;
import io.spring.infrastructure.mybatis.mapper.ArticleFavoriteMapper;
import io.spring.infrastructure.mybatis.mapper.ArticleMapper;
import io.spring.infrastructure.mybatis.mapper.CommentMapper;
import io.spring.infrastructure.mybatis.mapper.UserMapper;
import java.util.Optional;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Repository;
import org.springframework.transaction.annotation.Transactional;

@Repository
public class MyBatisUserRepository implements UserRepository {
private final UserMapper userMapper;
private final ArticleMapper articleMapper;
private final CommentMapper commentMapper;
private final ArticleFavoriteMapper articleFavoriteMapper;

@Autowired
public MyBatisUserRepository(UserMapper userMapper) {
public MyBatisUserRepository(
UserMapper userMapper,
ArticleMapper articleMapper,
CommentMapper commentMapper,
ArticleFavoriteMapper articleFavoriteMapper) {
this.userMapper = userMapper;
this.articleMapper = articleMapper;
this.commentMapper = commentMapper;
this.articleFavoriteMapper = articleFavoriteMapper;
}

@Override
Expand Down Expand Up @@ -57,4 +71,19 @@ public Optional<FollowRelation> findRelation(String userId, String targetId) {
public void removeRelation(FollowRelation followRelation) {
userMapper.deleteRelation(followRelation);
}

@Override
@Transactional
public void remove(User user) {
String userId = user.getId();
commentMapper.deleteByUserId(userId);
commentMapper.deleteByArticleUserId(userId);
articleFavoriteMapper.deleteByUserId(userId);
articleFavoriteMapper.deleteByArticleUserId(userId);
articleMapper.deleteArticleTagsByUserId(userId);
articleMapper.deleteByUserId(userId);
Comment on lines +79 to +84
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🔴 Comments by other users on deleted user's articles are not cleaned up, creating orphaned rows

The remove() method deletes comments made BY the user (commentMapper.deleteByUserId), but does not delete comments made by OTHER users on the deleted user's articles. When articleMapper.deleteByUserId(userId) later deletes the user's articles, those comments are left orphaned with article_id references pointing to non-existent articles.

Comparison with article_favorites handling

For article_favorites, the code correctly has two deletion steps:

  • articleFavoriteMapper.deleteByUserId(userId) — removes the user's own favorites
  • articleFavoriteMapper.deleteByArticleUserId(userId) — removes all favorites on the user's articles (ArticleFavoriteMapper.xml:13-15)

But for comments, only the first pattern is applied:

  • commentMapper.deleteByUserId(userId) — removes comments authored by the user
  • Missing: a deleteByArticleUserId equivalent to remove all comments on the user's articles

The orphaned comments can still be returned by CommentReadService.findById(id) (CommentReadService.xml:16-19) since that query only filters by comment id, not article id.

Prompt for agents
The remove() method in MyBatisUserRepository is missing deletion of comments by other users on the deleted user's articles. This parallels the pattern already used for article_favorites (deleteByArticleUserId).

To fix:
1. Add a new method to CommentMapper.java: void deleteByArticleUserId(@Param("userId") String userId)
2. Add a corresponding SQL in CommentMapper.xml: DELETE FROM comments WHERE article_id IN (SELECT id FROM articles WHERE user_id = #{userId})
3. Call commentMapper.deleteByArticleUserId(userId) in MyBatisUserRepository.remove(), after the existing commentMapper.deleteByUserId(userId) call and before articleMapper.deleteByUserId(userId)

This mirrors exactly what is already done for article_favorites with articleFavoriteMapper.deleteByArticleUserId(userId) at line 81.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good catch — fixed in 44857ee. Added commentMapper.deleteByArticleUserId(userId) to mirror the existing articleFavoriteMapper.deleteByArticleUserId() pattern. Also added a test assertion verifying comments by other users on the deleted user's articles are cleaned up.

userMapper.deleteFollowsByUserId(userId);
userMapper.deleteFollowsTargetingUser(userId);
userMapper.delete(userId);
}
}
6 changes: 6 additions & 0 deletions src/main/resources/mapper/ArticleFavoriteMapper.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@
<delete id="delete">
delete from article_favorites where article_id = #{favorite.articleId} and user_id = #{favorite.userId}
</delete>
<delete id="deleteByUserId">
delete from article_favorites where user_id = #{userId}
</delete>
<delete id="deleteByArticleUserId">
delete from article_favorites where article_id in (select id from articles where user_id = #{userId})
</delete>
<select id="find" resultMap="articleFavorite">
select
AF.article_id articleFavoriteArticleId,
Expand Down
6 changes: 6 additions & 0 deletions src/main/resources/mapper/ArticleMapper.xml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@
<delete id="delete">
delete from articles where id = #{id}
</delete>
<delete id="deleteArticleTagsByUserId">
delete from article_tags where article_id in (select id from articles where user_id = #{userId})
</delete>
<delete id="deleteByUserId">
delete from articles where user_id = #{userId}
</delete>
<sql id="selectArticle">
select
A.id articleId,
Expand Down
6 changes: 6 additions & 0 deletions src/main/resources/mapper/CommentMapper.xml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@
<delete id="delete">
delete from comments where id = #{id}
</delete>
<delete id="deleteByUserId">
delete from comments where user_id = #{userId}
</delete>
<delete id="deleteByArticleUserId">
delete from comments where article_id in (select id from articles where user_id = #{userId})
</delete>
<select id="findById" resultMap="comment">
select
id commentId,
Expand Down
9 changes: 9 additions & 0 deletions src/main/resources/mapper/UserMapper.xml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,15 @@
<delete id="deleteRelation">
delete from follows where user_id = #{followRelation.userId} and follow_id = #{followRelation.targetId}
</delete>
<delete id="delete">
delete from users where id = #{id}
</delete>
<delete id="deleteFollowsByUserId">
delete from follows where user_id = #{userId}
</delete>
<delete id="deleteFollowsTargetingUser">
delete from follows where follow_id = #{userId}
</delete>
<select id="findByUsername" resultMap="user">
select * from users where username = #{username}
</select>
Expand Down
1 change: 1 addition & 0 deletions src/main/resources/schema/schema.graphqls
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ type Mutation {
createUser(input: CreateUserInput): UserResult
login(password: String!, email: String!): UserPayload
updateUser(changes: UpdateUserInput!): UserPayload
deleteUser: DeletionStatus
followUser(username: String!): ProfilePayload
unfollowUser(username: String!): ProfilePayload

Expand Down
16 changes: 16 additions & 0 deletions src/test/java/io/spring/api/CurrentUserApiTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,22 @@ private HashMap<String, Object> prepareUpdateParam(
};
}

@Test
public void should_delete_current_user_account() throws Exception {
given()
.header("Authorization", "Token " + token)
.contentType("application/json")
.when()
.delete("/user")
.then()
.statusCode(204);
}

@Test
public void should_get_401_when_delete_without_token() throws Exception {
given().contentType("application/json").when().delete("/user").then().statusCode(401);
}

@Test
public void should_get_401_if_not_login() throws Exception {
given()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,38 @@
package io.spring.infrastructure.user;

import io.spring.core.article.Article;
import io.spring.core.article.ArticleRepository;
import io.spring.core.comment.Comment;
import io.spring.core.comment.CommentRepository;
import io.spring.core.favorite.ArticleFavorite;
import io.spring.core.favorite.ArticleFavoriteRepository;
import io.spring.core.user.FollowRelation;
import io.spring.core.user.User;
import io.spring.core.user.UserRepository;
import io.spring.infrastructure.DbTestBase;
import io.spring.infrastructure.repository.MyBatisArticleFavoriteRepository;
import io.spring.infrastructure.repository.MyBatisArticleRepository;
import io.spring.infrastructure.repository.MyBatisCommentRepository;
import io.spring.infrastructure.repository.MyBatisUserRepository;
import java.util.Arrays;
import java.util.Optional;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Import;

@Import(MyBatisUserRepository.class)
@Import({
MyBatisUserRepository.class,
MyBatisArticleRepository.class,
MyBatisCommentRepository.class,
MyBatisArticleFavoriteRepository.class
})
public class MyBatisUserRepositoryTest extends DbTestBase {
@Autowired private UserRepository userRepository;
@Autowired private ArticleRepository articleRepository;
@Autowired private CommentRepository commentRepository;
@Autowired private ArticleFavoriteRepository articleFavoriteRepository;
private User user;

@BeforeEach
Expand Down Expand Up @@ -70,4 +88,34 @@ public void should_unfollow_user_success() {
userRepository.removeRelation(followRelation);
Assertions.assertFalse(userRepository.findRelation(user.getId(), other.getId()).isPresent());
}

@Test
public void should_remove_user_and_cascade_related_data() {
userRepository.save(user);

Article article =
new Article("test", "desc", "body", Arrays.asList("java"), user.getId());
articleRepository.save(article);

Comment comment = new Comment("nice article", user.getId(), article.getId());
commentRepository.save(comment);

ArticleFavorite favorite = new ArticleFavorite(article.getId(), user.getId());
articleFavoriteRepository.save(favorite);

User other = new User("other@example.com", "other", "123", "", "");
userRepository.save(other);
userRepository.saveRelation(new FollowRelation(user.getId(), other.getId()));

Comment otherComment = new Comment("great post", other.getId(), article.getId());
commentRepository.save(otherComment);

userRepository.remove(user);

Assertions.assertFalse(userRepository.findById(user.getId()).isPresent());
Assertions.assertFalse(articleRepository.findById(article.getId()).isPresent());
Assertions.assertFalse(userRepository.findRelation(user.getId(), other.getId()).isPresent());
Assertions.assertFalse(
commentRepository.findById(article.getId(), otherComment.getId()).isPresent());
}
}