Issue53/sonar findings (#56)

* refactor CardService and start writing tests

* autowire mongotemplate

* complete CategoryServiceIT

* solve testcontainer issue and start of Controller-IT

* finish CategoryControllerIT

* add Mapper test - category package complete  now

server/src/main/java/org/hyperskill/community/flashcards/card/ExampleDataInitializer.java

@@ -32,10 +32,10 @@
 @RequiredArgsConstructor
 @Slf4j
 public class ExampleDataInitializer {
-    private static final String exampleCollection = "example";
-    private static final String userCollection = "user";
-    private static final String categoryCollection = "category";
-    private static final String userJsonPath = "/json/users.json";
+    private static final String EXAMPLE = "example";
+    private static final String USER = "user";
+    private static final String CATEGORY = "category";
+    private static final String USER_JSON_PATH = "/json/users.json";
     private static final String flashcardsJsonPath = "/json/flashcards.json";
     private final ObjectMapper objectMapper;
     private final UserMapper userMapper;
@@ -51,20 +51,20 @@ public class ExampleDataInitializer {
     public void init() {
         log.info("Inserting sample data to the database...");
 
-        if (isCollectionNotEmpty(userCollection)) {
-            log.warn("Collection {} is not empty, aborting database initialization", userCollection);
+        if (isCollectionNotEmpty(USER)) {
+            log.warn("Collection {} is not empty, aborting database initialization", USER);
             return;
         }
-        if (isCollectionNotEmpty(categoryCollection)) {
-            log.warn("Collection {} is not empty, aborting database initialization", categoryCollection);
+        if (isCollectionNotEmpty(CATEGORY)) {
+            log.warn("Collection {} is not empty, aborting database initialization", CATEGORY);
             return;
         }
-        if (isCollectionNotEmpty(exampleCollection)) {
-            log.warn("Collection {} is not empty, aborting database initialization", exampleCollection);
+        if (isCollectionNotEmpty(EXAMPLE)) {
+            log.warn("Collection {} is not empty, aborting database initialization", EXAMPLE);
             return;
         }
 
-        Resource usersJson = new ClassPathResource(userJsonPath);
+        Resource usersJson = new ClassPathResource(USER_JSON_PATH);
         Resource flashcardsJson = new ClassPathResource(flashcardsJsonPath);
 
         try {
@@ -77,17 +77,17 @@ public void init() {
 
             log.info("Sample users successfully inserted!");
 
-            var categoryAccess = new CategoryAccess(users.get(0).getUsername(), "rwd");
-            mongoTemplate.insert(new Category(null, exampleCollection, Set.of(categoryAccess)));
+            var categoryAccess = new CategoryAccess(users.getFirst().getUsername(), "rwd");
+            mongoTemplate.insert(new Category(null, EXAMPLE, Set.of(categoryAccess)));
 
             JsonNode jsonNode = objectMapper.readTree(flashcardsJson.getFile());
             List<SingleChoiceQuiz> scqCards = parseCards(jsonNode.get("scq_cards"), SingleChoiceQuiz.class);
             List<MultipleChoiceQuiz> mcqCards = parseCards(jsonNode.get("mcq_cards"), MultipleChoiceQuiz.class);
             List<QuestionAndAnswer> qnaCards = parseCards(jsonNode.get("qna_cards"), QuestionAndAnswer.class);
 
-            mongoTemplate.insert(scqCards, exampleCollection);
-            mongoTemplate.insert(mcqCards, exampleCollection);
-            mongoTemplate.insert(qnaCards, exampleCollection);
+            mongoTemplate.insert(scqCards, EXAMPLE);
+            mongoTemplate.insert(mcqCards, EXAMPLE);
+            mongoTemplate.insert(qnaCards, EXAMPLE);
 
             log.info("Sample flashcards successfully inserted!");
 

server/src/main/java/org/hyperskill/community/flashcards/category/CategoryService.java

@@ -11,45 +11,38 @@
 import org.hyperskill.community.flashcards.common.exception.ResourceNotFoundException;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.PageImpl;
-import org.springframework.data.domain.Pageable;
+import org.springframework.data.domain.PageRequest;
 import org.springframework.data.domain.Sort;
 import org.springframework.data.mongodb.core.MongoTemplate;
-import org.springframework.data.mongodb.core.aggregation.Aggregation;
 import org.springframework.data.mongodb.core.query.Criteria;
 import org.springframework.data.mongodb.core.query.Query;
 import org.springframework.security.access.AccessDeniedException;
 import org.springframework.stereotype.Service;
 
-import static org.springframework.data.mongodb.core.query.Update.update;
-
 import java.util.Optional;
 import java.util.Set;
 
+import static org.springframework.data.mongodb.core.query.Update.update;
+
 @Service
 @RequiredArgsConstructor
 @Slf4j
 public class CategoryService {
-    private static final String collection = "category";
-    private static final int pageSize = 20;
+    private static final String CATEGORY = "category";
+    private static final int PAGE_SIZE = 20;
+
     private final MongoTemplate mongoTemplate;
 
     public Page<Category> getCategories(String username, int page) {
         var usernameHasReadPermission = Criteria.where("username").is(username)
-                .and("permission").regex("(.*r.*){1,3}");
-        var currentUserHasAccess = Criteria.where("access").elemMatch(usernameHasReadPermission);
+                .and("permission").regex(".*r.*");
+        var categoriesQuery = new Query(Criteria.where("access").elemMatch(usernameHasReadPermission));
 
-        var count = mongoTemplate.count(new Query(currentUserHasAccess), collection);
+        var count = mongoTemplate.count(categoriesQuery, CATEGORY);
+        var pageRequest = PageRequest.of(page, PAGE_SIZE, Sort.by("name"));
+        var categories = mongoTemplate.find(categoriesQuery.with(pageRequest), Category.class, CATEGORY);
 
-        var aggregation = Aggregation.newAggregation(
-                Aggregation.match(currentUserHasAccess),
-                Aggregation.sort(Sort.by("name")),
-                Aggregation.skip((long) page * pageSize),
-                Aggregation.limit(pageSize)
-        );
-
-        var categories = mongoTemplate.aggregate(aggregation, collection, Category.class).getMappedResults();
-
-        return new PageImpl<>(categories, Pageable.ofSize(pageSize), count);
+        return new PageImpl<>(categories, pageRequest, count);
     }
 
     public Category findById(String username, String categoryId) {
@@ -59,7 +52,6 @@ public Category findById(String username, String categoryId) {
     public Category findById(String username, String categoryId, String permission) {
         var category = Optional.ofNullable(mongoTemplate.findById(categoryId, Category.class))
                 .orElseThrow(ResourceNotFoundException::new);
-
         assertCanAccess(username, category, permission);
 
         return category;
@@ -68,38 +60,28 @@ public Category findById(String username, String categoryId, String permission)
     public String createCategory(String username, CategoryCreateRequest request) {
         // we assume that category names are unique which is still subject to discussion
         var categoryName = request.name();
-
-        var query = Query.query(Criteria.where("name").is(categoryName));
-        var category = mongoTemplate.findOne(query, Category.class, "category");
-        if (category != null) {
-            throw new ResourceAlreadyExistsException();
-        }
+        throwIfCategoryExists(categoryName);
 
         var access = new CategoryAccess(username, "rwd");
         var newCategory = new Category(null, categoryName, Set.of(access));
-        newCategory = mongoTemplate.insert(newCategory, "category");
+        newCategory = mongoTemplate.insert(newCategory, CATEGORY);
         mongoTemplate.getDb().createCollection(categoryName);
 
         return newCategory.id();
     }
 
     public void deleteById(String username, String categoryId) {
-        var category = findById(username, categoryId);
-        assertCanAccess(username, category, "d");
-
-        mongoTemplate.remove(category, "category");
+        var category = findById(username, categoryId, "d");
+        mongoTemplate.remove(category, CATEGORY);
         mongoTemplate.dropCollection(category.name());
     }
 
     public Category updateById(String username, String categoryId, CategoryUpdateRequest request) {
         // find if the requested collection exists and can be modified
-        var category = findById(username, categoryId);
-        assertCanAccess(username, category, "w");
+        var category = findById(username, categoryId, "w");
 
         // check if the new name is already taken
-        if (mongoTemplate.getCollectionNames().contains(request.name())) {
-            throw new ResourceAlreadyExistsException();
-        }
+        throwIfCategoryExists(request.name());
 
         // rename the existing collection
         var namespace = new MongoNamespace(mongoTemplate.getDb().getName(), request.name());
@@ -113,11 +95,17 @@ public Category updateById(String username, String categoryId, CategoryUpdateReq
         return mongoTemplate.findOne(query, Category.class);
     }
 
+    private void throwIfCategoryExists(String name) {
+        if (mongoTemplate.getCollectionNames().contains(name)) {
+            throw new ResourceAlreadyExistsException();
+        }
+    }
+
     private void assertCanAccess(String username, Category category, String permission) {
-        var regex = "(.*%s.*){1,3}".formatted(permission);
-        category.access().stream()
-                .filter(access -> access.username().equals(username) && access.permission().matches(regex))
-                .findFirst()
-                .orElseThrow(() -> new AccessDeniedException("Access '%s' denied".formatted(permission)));
+        if (category.access().stream()
+                .noneMatch(access -> access.username().equals(username)
+                                     && access.permission().contains(permission))) {
+            throw new AccessDeniedException("Access '%s' denied".formatted(permission));
+        }
     }
 }

server/src/main/java/org/hyperskill/community/flashcards/category/mapper/CategoryMapper.java

@@ -13,6 +13,7 @@
 @Slf4j
 public class CategoryMapper {
     private final String currentUserName;
+
     public CategoryMapper(String currentUserName) {
         Objects.requireNonNull(currentUserName, "Username cannot be null");
         this.currentUserName = currentUserName;

server/src/main/java/org/hyperskill/community/flashcards/config/MongoConfiguration.java

@@ -1,15 +1,13 @@
 package org.hyperskill.community.flashcards.config;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.mongodb.client.MongoClients;
 import org.bson.Document;
 import org.hyperskill.community.flashcards.card.mapper.CardReadConverter;
 import org.hyperskill.community.flashcards.card.model.Card;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.core.convert.converter.Converter;
 import org.springframework.data.mongodb.config.EnableMongoAuditing;
-import org.springframework.data.mongodb.core.MongoTemplate;
 import org.springframework.data.mongodb.core.convert.MongoCustomConversions;
 
 import java.util.ArrayList;
@@ -19,11 +17,6 @@
 @EnableMongoAuditing
 public class MongoConfiguration {
 
-    @Bean
-    public MongoTemplate mongoTemplate() {
-        return new MongoTemplate(MongoClients.create(), "cards");
-    }
-
     @Bean
     public MongoCustomConversions mongoCustomConversions(ObjectMapper objectMapper) {
         List<Converter<Document, Card>> converters = new ArrayList<>();

server/src/test/java/org/hyperskill/community/flashcards/TestMongoConfiguration.java

@@ -4,19 +4,13 @@
 import org.springframework.boot.testcontainers.service.connection.ServiceConnection;
 import org.springframework.context.annotation.Bean;
 import org.testcontainers.containers.MongoDBContainer;
-import org.testcontainers.utility.TestcontainersConfiguration;
-
-import java.util.List;
 
 @TestConfiguration(proxyBeanMethods = false)
 public class TestMongoConfiguration {
 
     @Bean
     @ServiceConnection
     public MongoDBContainer mongoDBContainer() {
-        TestcontainersConfiguration.getInstance().updateUserConfig("testcontainers.reuse.enable", "true");
-        var container = new MongoDBContainer("mongo:7.0.4-jammy").withReuse(true);
-        container.setPortBindings(List.of("27017:27017"));
-        return container;
+        return new MongoDBContainer("mongo:7.0.4-jammy");
     }
 }

server/src/test/java/org/hyperskill/community/flashcards/category/CategoryControllerValidationUnitTest.java

@@ -0,0 +1,51 @@
+package org.hyperskill.community.flashcards.category;
+
+import jakarta.validation.Validation;
+import jakarta.validation.Validator;
+import org.hyperskill.community.flashcards.category.request.CategoryCreateRequest;
+import org.hyperskill.community.flashcards.category.request.CategoryUpdateRequest;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class CategoryControllerValidationUnitTest {
+
+    Validator validator;
+
+    @BeforeEach
+    void setup() {
+        try (var validatorFactory = Validation.buildDefaultValidatorFactory()) {
+            this.validator = validatorFactory.getValidator();
+        }
+    }
+
+    @Test
+    void whenNullName_ValidationError()  {
+        var createRequest = new CategoryCreateRequest(null);
+        assertFalse(validator.validate(createRequest).isEmpty());
+        var updateRequest = new CategoryUpdateRequest(null);
+        assertFalse(validator.validate(updateRequest).isEmpty());
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = {"", " ", "  "})
+    void whenBlankName_ValidationError(String name)  {
+        var createRequest = new CategoryCreateRequest(name);
+        assertFalse(validator.validate(createRequest).isEmpty());
+        var updateRequest = new CategoryUpdateRequest(name);
+        assertFalse(validator.validate(updateRequest).isEmpty());
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = {"a", "category", "a+-3§"})
+    void whenNonBlankName_ValidationOk(String name)  {
+        var createRequest = new CategoryCreateRequest(name);
+        assertTrue(validator.validate(createRequest).isEmpty());
+        var updateRequest = new CategoryUpdateRequest(name);
+        assertTrue(validator.validate(updateRequest).isEmpty());
+    }
+}

server/src/test/java/org/hyperskill/community/flashcards/category/CategoryMapperTest.java

@@ -0,0 +1,48 @@
+package org.hyperskill.community.flashcards.category;
+
+import org.hyperskill.community.flashcards.category.mapper.CategoryMapper;
+import org.hyperskill.community.flashcards.category.model.Category;
+import org.hyperskill.community.flashcards.category.model.CategoryAccess;
+import org.hyperskill.community.flashcards.common.response.ActionType;
+import org.hyperskill.community.flashcards.common.response.PermittedAction;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.util.Set;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+class CategoryMapperTest {
+
+    CategoryMapper mapper;
+
+    @BeforeEach
+    void setUp() {
+        mapper = new CategoryMapper("user");
+    }
+
+    @Test
+    void permittedUser_mapsCorrectly() {
+        var category = createCategory("user", "rwd");
+        var dto = mapper.categoryToCategoryDto(category);
+        assertEquals(category.id(), dto.id());
+        assertEquals(category.name(), dto.name());
+        var expectedPermissions = Set.of(
+                new PermittedAction(ActionType.READ, "/api/categories/12345"),
+                new PermittedAction(ActionType.WRITE, "/api/categories/12345"),
+                new PermittedAction(ActionType.DELETE, "/api/categories/12345"));
+        assertEquals(expectedPermissions, dto.actions());
+    }
+
+    @Test
+    void notPermittedUser_mapThrows() {
+        var category = createCategory("other", "rw");
+        assertThrows(IllegalStateException.class, () -> mapper.categoryToCategoryDto(category));
+    }
+
+    private Category createCategory( String username, String permissions) {
+        return new Category("12345", "test", Set.of(new CategoryAccess(username, permissions)));
+    }
+
+}