refactor: architecture

ThisAster · ThisAster · commit b8ffdb974fb9 · 2025-05-15T19:18:55.000+03:00
remove the double relation between subscription and user
diff --git a/src/main/java/com/thisaster/testtask/subscription/entity/Subscription.java b/src/main/java/com/thisaster/testtask/subscription/entity/Subscription.java
@@ -1,22 +1,17 @@
 package com.thisaster.testtask.subscription.entity;
 
-import com.thisaster.testtask.user.entity.User;
 import jakarta.persistence.Column;
 import jakarta.persistence.Entity;
 import jakarta.persistence.GeneratedValue;
 import jakarta.persistence.GenerationType;
 import jakarta.persistence.Id;
-import jakarta.persistence.ManyToMany;
 import jakarta.persistence.Table;
 import lombok.AllArgsConstructor;
 import lombok.Builder;
 import lombok.Getter;
 import lombok.NoArgsConstructor;
 import lombok.Setter;
 
-import java.util.HashSet;
-import java.util.Set;
-
 @Builder
 @Entity
 @Table(name = "subscriptions")
@@ -27,10 +22,7 @@
 public class Subscription {
     @Id
     @GeneratedValue(strategy = GenerationType.IDENTITY)
-    private long id;
+    private Long id;
     @Column(unique = true, nullable = false)
     private String name;
-    @ManyToMany(mappedBy = "subscriptions")
-    @Builder.Default
-    private Set<User> users = new HashSet<>();
 }
diff --git a/src/main/java/com/thisaster/testtask/subscription/mapper/SubscriptionMapper.java b/src/main/java/com/thisaster/testtask/subscription/mapper/SubscriptionMapper.java
@@ -3,7 +3,6 @@
 import com.thisaster.testtask.subscription.dto.SubscriptionDTO;
 import com.thisaster.testtask.subscription.entity.Subscription;
 import org.mapstruct.Mapper;
-import org.mapstruct.Mapping;
 import org.mapstruct.MappingConstants;
 
 import java.util.Set;
@@ -13,7 +12,6 @@ public abstract class SubscriptionMapper {
 
     public abstract SubscriptionDTO toDTO(Subscription sub);
 
-    @Mapping(target = "users", ignore = true)
     public abstract Subscription toEntity(SubscriptionDTO subDTO);
 
     public abstract Set<SubscriptionDTO> toDTOSet(Set<Subscription> subscriptions);
diff --git a/src/main/java/com/thisaster/testtask/subscription/repository/SubscriptionRepository.java b/src/main/java/com/thisaster/testtask/subscription/repository/SubscriptionRepository.java
@@ -1,7 +1,6 @@
 package com.thisaster.testtask.subscription.repository;
 
 import com.thisaster.testtask.subscription.entity.Subscription;
-import com.thisaster.testtask.user.entity.User;
 import org.springframework.data.jpa.repository.JpaRepository;
 import org.springframework.data.jpa.repository.Query;
 import org.springframework.stereotype.Repository;
@@ -11,19 +10,15 @@
 
 @Repository
 public interface SubscriptionRepository extends JpaRepository<Subscription, Long> {
-    @Query(value = "SELECT s.id, s.name, COUNT(u.id) as user_count " +
+    @Query(value = "SELECT s.id as id, s.name as name, COUNT(u.id) as userCount " +
             "FROM subscriptions s " +
             "LEFT JOIN user_subscription us ON s.id = us.subscription_id " +
             "LEFT JOIN user_ u ON us.user_id = u.id " +
-            "GROUP BY s.id " +
-            "ORDER BY user_count DESC " +
+            "GROUP BY s.id, s.name " +
+            "ORDER BY userCount DESC " +
             "LIMIT 3", nativeQuery = true)
     Set<Subscription> findTop3ByNameOrderByUsersCountDesc();
 
-
-    @Query("SELECT s FROM Subscription s JOIN s.users u WHERE u = :user")
-    Set<Subscription> findSubscriptionsByUser(User user);
-
     @Query("SELECT s FROM Subscription s WHERE s.name = :name")
     Optional<Subscription> findByName(String name);
 
diff --git a/src/main/java/com/thisaster/testtask/user/controller/UserController.java b/src/main/java/com/thisaster/testtask/user/controller/UserController.java
@@ -6,19 +6,11 @@
 import com.thisaster.testtask.user.dto.UserDTO;
 import com.thisaster.testtask.user.entity.User;
 import com.thisaster.testtask.user.mapper.UserMapper;
-import com.thisaster.testtask.user.service.RoleService;
 import com.thisaster.testtask.user.service.UserService;
 import lombok.RequiredArgsConstructor;
 import org.springframework.http.ResponseEntity;
 import org.springframework.validation.annotation.Validated;
-import org.springframework.web.bind.annotation.DeleteMapping;
-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.PutMapping;
-import org.springframework.web.bind.annotation.RequestBody;
-import org.springframework.web.bind.annotation.RequestMapping;
-import org.springframework.web.bind.annotation.RestController;
+import org.springframework.web.bind.annotation.*;
 
 import java.util.Set;
 
@@ -29,7 +21,6 @@ public class UserController {
     private final UserService userService;
     private final UserMapper userMapper;
     private final SubscriptionMapper subscriptionMapper;
-    private final RoleService roleService;
 
     @GetMapping("/{id}")
     public ResponseEntity<UserDTO> getUserInfo(@PathVariable Long id) {
@@ -47,8 +38,6 @@ public ResponseEntity<String> deleteUser(@PathVariable Long id) {
     @PutMapping("/{id}")
     public ResponseEntity<UserDTO> updateUser(@PathVariable Long id, @RequestBody @Validated UserDTO userDTO) {
         User user = userMapper.toEntity(userDTO);
-        user.setRole(roleService.getByRoleName(userDTO.getRole()));
-
         User updatedUser = userService.updateUser(id, user);
         UserDTO updatedDTO = userMapper.toDTO(updatedUser);
         return ResponseEntity.ok(updatedDTO);
diff --git a/src/main/java/com/thisaster/testtask/user/dto/UserDTO.java b/src/main/java/com/thisaster/testtask/user/dto/UserDTO.java
@@ -34,7 +34,9 @@ public class UserDTO {
     @JsonProperty(access = JsonProperty.Access.READ_WRITE)
     private Set<SubscriptionDTO> subscriptions;
 
-    @JsonProperty(access = JsonProperty.Access.WRITE_ONLY)
+    @JsonProperty(access = JsonProperty.Access.READ_WRITE)
+
+    @NotBlank(message = "Role cannot be blank")
     @Pattern(regexp = "^(ADMIN|USER|SUPERVISOR)$", message = "Role must be either admin or user")
     private String role;
 }
diff --git a/src/main/java/com/thisaster/testtask/user/mapper/RoleMapper.java b/src/main/java/com/thisaster/testtask/user/mapper/RoleMapper.java
@@ -0,0 +1,30 @@
+package com.thisaster.testtask.user.mapper;
+
+import com.thisaster.testtask.user.entity.RoleEntity;
+import com.thisaster.testtask.user.service.RoleService;
+import org.mapstruct.Mapper;
+import org.mapstruct.MappingConstants;
+import org.mapstruct.Named;
+import org.springframework.beans.factory.annotation.Autowired;
+
+@Mapper(componentModel = MappingConstants.ComponentModel.SPRING)
+public abstract class RoleMapper {
+
+    @Autowired
+    private RoleService roleService;
+
+    @Named("stringToEntity")
+    public RoleEntity fromString(String roleName) {
+        if (roleName == null) {
+            return null;
+        }
+        return roleService.getByRoleName(roleName);
+    }
+    @Named("entityToString")
+    public String toString(RoleEntity entity) {
+        if (entity == null) {
+            return null;
+        }
+        return entity.getName();
+    }
+}
diff --git a/src/main/java/com/thisaster/testtask/user/mapper/UserMapper.java b/src/main/java/com/thisaster/testtask/user/mapper/UserMapper.java
@@ -8,13 +8,13 @@
 import org.mapstruct.MappingConstants;
 
 @Mapper(componentModel = MappingConstants.ComponentModel.SPRING,
-        uses = {SubscriptionMapper.class})
-public abstract class UserMapper {
+        uses = {SubscriptionMapper.class, RoleMapper.class})
+public interface UserMapper {
 
-    @Mapping(target = "role", source = "role.name")
-    public abstract UserDTO toDTO(User user);
+    @Mapping(target = "role", qualifiedByName = "entityToString", source = "role")
+    UserDTO toDTO(User user);
 
-    @Mapping(target = "role", ignore = true) // всё равно подставим вручную в контроллере
-    @Mapping(target = "roleId", ignore = true) // подставим вручную
-    public abstract User toEntity(UserDTO userDTO);
+    @Mapping(target = "role", qualifiedByName = "stringToEntity", source = "role")
+    @Mapping(target = "roleId", expression = "java(roleMapper.fromString(userDTO.getRole()).getId())")
+    User toEntity(UserDTO userDTO);
 }
diff --git a/src/main/java/com/thisaster/testtask/user/repository/UserRepository.java b/src/main/java/com/thisaster/testtask/user/repository/UserRepository.java
@@ -12,4 +12,6 @@ public interface UserRepository extends JpaRepository<User, Long> {
 
     @Query("SELECT u FROM User u WHERE u.username = ?1")
     Optional<User> findByUsername(String username);
+
+    int countBySubscriptions_Id(Long subscriptionId);
 }
diff --git a/src/main/java/com/thisaster/testtask/user/service/UserService.java b/src/main/java/com/thisaster/testtask/user/service/UserService.java
@@ -68,34 +68,34 @@ public void subscribeUserToSub(Long userId, Subscription subscription) {
         }
 
         boolean addedToUser = user.getSubscriptions().add(existingSubscription);
-        boolean addedToSub = existingSubscription.getUsers().add(user);
 
-        if (addedToUser || addedToSub) {
+        if (addedToUser) {
             userRepository.save(user);
-            subscriptionRepository.save(existingSubscription);
         }
     }
 
     @Transactional
     public void removeSubFromUser(Long userId, Long subscriptionId) {
-        User user = userRepository.findById(userId).orElseThrow(()
-                -> new EntityNotFoundException("User not found with id: " + userId));
+        User user = userRepository.findById(userId)
+                .orElseThrow(() -> new EntityNotFoundException("User not found with id: " + userId));
 
         Subscription subscription = subscriptionRepository.findById(subscriptionId)
                 .orElseThrow(() -> new EntityNotFoundException("Subscription not found with id: " + subscriptionId));
 
-        subscription.getUsers().remove(user);
-        if (subscription.getUsers().isEmpty()) {
+        boolean removed = user.getSubscriptions().remove(subscription);
+        if (removed) userRepository.save(user);
+
+        int count = userRepository.countBySubscriptions_Id(subscriptionId);
+
+        if (count == 0) {
             subscriptionRepository.delete(subscription);
         }
-        user.getSubscriptions().remove(subscription);
-        userRepository.save(user);
     }
 
     public Set<Subscription> getUserSubscriptions(Long userId) {
         User user = userRepository.findById(userId)
                 .orElseThrow(() -> new EntityNotFoundException("User not found with id: " + userId));
-        return subscriptionRepository.findSubscriptionsByUser(user);
+        return user.getSubscriptions();
     }
 
     public void deleteUser(Long userId) {
@@ -114,6 +114,7 @@ private void updateUserDetails(User existingUser, User newUser) {
         }
 
         existingUser.setRole(roleService.getByRoleId(newUser.getRoleId()));
+        existingUser.setRoleId(newUser.getRoleId());
     }
 
     @Transactional
diff --git a/src/test/java/com/thisaster/testtask/user/controller/UserControllerTest.java b/src/test/java/com/thisaster/testtask/user/controller/UserControllerTest.java
@@ -24,6 +24,7 @@
 import static org.hamcrest.core.StringContains.containsString;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
 
 @Slf4j
 @AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE)
@@ -94,9 +95,7 @@ void shouldGetUserSubscriptions() throws Exception {
         mockMvc.perform(get("/api/users/{id}/subscriptions", userId)
                         .header("Authorization", "Bearer " + token))
                 .andExpect(status().isOk())
-                .andExpect(jsonPath("$[0].name", is("Premium Plan")))
-                .andExpect(jsonPath("$[1].name", is("Newsletter")))
-                .andExpect(jsonPath("$[2].name", is("Basic Access")));
+                .andExpect(jsonPath("$[*].name", containsInAnyOrder("Premium Plan", "Newsletter", "Basic Access")));
     }
 
     @Test
@@ -115,7 +114,7 @@ void shouldDeleteUserSubscription() throws Exception {
     @Order(6)
     void shouldUpdateUser(@Value("classpath:user/updateUser.json") Resource json) throws Exception {
         Long userId = 3L;
-        mockMvc.perform(put("/api/users/{userId}", userId)
+        MvcResult result = mockMvc.perform(put("/api/users/{userId}", userId)
                         .header("Authorization", "Bearer " + token)
                         .contentType(MediaType.APPLICATION_JSON)
                         .content(json.getContentAsByteArray()))
@@ -127,7 +126,11 @@ void shouldUpdateUser(@Value("classpath:user/updateUser.json") Resource json) th
                 .andExpect(jsonPath("$.subscriptions", hasSize(3)))
                 .andExpect(jsonPath("$.subscriptions[*].name", containsInAnyOrder(
                         "Premium Plan", "Newsletter", "Yandex Premium"
-                )));
+                )))
+                .andExpect(jsonPath("$.role", is("SUPERVISOR")))
+                .andReturn();
+
+        System.out.println("Result : " + result.getResponse().getContentAsString());
 
         contains(subscriptionRepository.existsByName("Yandex Premium"));
     }
diff --git a/src/test/resources/user/updateUser.json b/src/test/resources/user/updateUser.json
@@ -12,5 +12,6 @@
     {
       "name": "Yandex Premium"
     }
-  ]
+  ],
+  "role": "SUPERVISOR"
 }

Original file line number	Diff line number	Diff line change
`@@ -12,4 +12,6 @@ public interface UserRepository extends JpaRepository<User, Long> {`
`12`	`12`
`13`	`13`	`@Query("SELECT u FROM User u WHERE u.username = ?1")`
`14`	`14`	`Optional<User> findByUsername(String username);`
	`15`	`+`
	`16`	`+ int countBySubscriptions_Id(Long subscriptionId);`
`15`	`17`	`}`
Original file line number	Diff line number	Diff line change
`@@ -68,34 +68,34 @@ public void subscribeUserToSub(Long userId, Subscription subscription) {`
`68`	`68`	`}`
`69`	`69`
`70`	`70`	`boolean addedToUser = user.getSubscriptions().add(existingSubscription);`
`71`		`- boolean addedToSub = existingSubscription.getUsers().add(user);`
`72`	`71`
`73`		`- if (addedToUser \|\| addedToSub) {`
	`72`	`+ if (addedToUser) {`
`74`	`73`	`userRepository.save(user);`
`75`		`- subscriptionRepository.save(existingSubscription);`
`76`	`74`	`}`
`77`	`75`	`}`
`78`	`76`
`79`	`77`	`@Transactional`
`80`	`78`	`public void removeSubFromUser(Long userId, Long subscriptionId) {`
`81`		`- User user = userRepository.findById(userId).orElseThrow(()`
`82`		`- -> new EntityNotFoundException("User not found with id: " + userId));`
	`79`	`+ User user = userRepository.findById(userId)`
	`80`	`+ .orElseThrow(() -> new EntityNotFoundException("User not found with id: " + userId));`
`83`	`81`
`84`	`82`	`Subscription subscription = subscriptionRepository.findById(subscriptionId)`
`85`	`83`	`.orElseThrow(() -> new EntityNotFoundException("Subscription not found with id: " + subscriptionId));`
`86`	`84`
`87`		`- subscription.getUsers().remove(user);`
`88`		`- if (subscription.getUsers().isEmpty()) {`
	`85`	`+ boolean removed = user.getSubscriptions().remove(subscription);`
	`86`	`+ if (removed) userRepository.save(user);`
	`87`	`+`
	`88`	`+ int count = userRepository.countBySubscriptions_Id(subscriptionId);`
	`89`	`+`
	`90`	`+ if (count == 0) {`
`89`	`91`	`subscriptionRepository.delete(subscription);`
`90`	`92`	`}`
`91`		`- user.getSubscriptions().remove(subscription);`
`92`		`- userRepository.save(user);`
`93`	`93`	`}`
`94`	`94`
`95`	`95`	`public Set<Subscription> getUserSubscriptions(Long userId) {`
`96`	`96`	`User user = userRepository.findById(userId)`
`97`	`97`	`.orElseThrow(() -> new EntityNotFoundException("User not found with id: " + userId));`
`98`		`- return subscriptionRepository.findSubscriptionsByUser(user);`
	`98`	`+ return user.getSubscriptions();`
`99`	`99`	`}`
`100`	`100`
`101`	`101`	`public void deleteUser(Long userId) {`
`@@ -114,6 +114,7 @@ private void updateUserDetails(User existingUser, User newUser) {`
`114`	`114`	`}`
`115`	`115`
`116`	`116`	`existingUser.setRole(roleService.getByRoleId(newUser.getRoleId()));`
	`117`	`+ existingUser.setRoleId(newUser.getRoleId());`
`117`	`118`	`}`
`118`	`119`
`119`	`120`	`@Transactional`
Original file line number	Diff line number	Diff line change
`@@ -12,5 +12,6 @@`
`12`	`12`	`{`
`13`	`13`	`"name": "Yandex Premium"`
`14`	`14`	`}`
`15`		`- ]`
	`15`	`+ ],`
	`16`	`+ "role": "SUPERVISOR"`
`16`	`17`	`}`