refactor(server): dumb down code: don't return Optional for update op…

…erations in store (#455) The resource should have been validated beforehand in get operations, so it is redundant.
ia-toki · Jun 22, 2023 · fb425ef · fb425ef
1 parent b2008d9
commit fb425ef
Show file tree

Hide file tree

Showing 17 changed files with 126 additions and 138 deletions.
diff --git a/...-backends/judgels-server-app/src/main/java/judgels/jerahmeel/archive/ArchiveResource.java b/...-backends/judgels-server-app/src/main/java/judgels/jerahmeel/archive/ArchiveResource.java
@@ -53,8 +53,9 @@ public Archive createArchive(AuthHeader authHeader, ArchiveCreateData data) {
     @UnitOfWork
     public Archive updateArchive(AuthHeader authHeader, String archiveJid, ArchiveUpdateData data) {
         String actorJid = actorChecker.check(authHeader);
+        checkFound(archiveStore.getArchiveByJid(archiveJid));
         checkAllowed(roleChecker.isAdmin(actorJid));
 
-        return checkFound(archiveStore.updateArchive(archiveJid, data));
+        return archiveStore.updateArchive(archiveJid, data);
     }
 }
diff --git a/...els-backends/judgels-server-app/src/main/java/judgels/jerahmeel/archive/ArchiveStore.java b/...els-backends/judgels-server-app/src/main/java/judgels/jerahmeel/archive/ArchiveStore.java
@@ -38,6 +38,10 @@ public List<Archive> getArchives() {
                 .collect(toList());
     }
 
+    public Optional<Archive> getArchiveByJid(String archiveJid) {
+        return archiveDao.selectByJid(archiveJid).map(ArchiveStore::fromModel);
+    }
+
     public Optional<Archive> getArchiveBySlug(String archiveSlug) {
         return archiveDao.selectBySlug(archiveSlug).map(ArchiveStore::fromModel);
     }
@@ -61,23 +65,22 @@ public Archive createArchive(ArchiveCreateData data) {
         return fromModel(archiveDao.insert(model));
     }
 
-    public Optional<Archive> updateArchive(String archiveJid, ArchiveUpdateData data) {
-        return archiveDao.selectByJid(archiveJid).map(model -> {
-            if (data.getSlug().isPresent()) {
-                String newSlug = data.getSlug().get();
-                if (model.slug == null || !model.slug.equals(newSlug)) {
-                    if (archiveDao.selectBySlug(newSlug).isPresent()) {
-                        throw ArchiveErrors.slugAlreadyExists(newSlug);
-                    }
+    public Archive updateArchive(String archiveJid, ArchiveUpdateData data) {
+        ArchiveModel model = archiveDao.findByJid(archiveJid);
+        if (data.getSlug().isPresent()) {
+            String newSlug = data.getSlug().get();
+            if (model.slug == null || !model.slug.equals(newSlug)) {
+                if (archiveDao.selectBySlug(newSlug).isPresent()) {
+                    throw ArchiveErrors.slugAlreadyExists(newSlug);
                 }
             }
+        }
 
-            data.getSlug().ifPresent(slug -> model.slug = slug);
-            data.getName().ifPresent(name -> model.name = name);
-            data.getCategory().ifPresent(category -> model.category = category);
-            data.getDescription().ifPresent(description -> model.description = description);
-            return fromModel(archiveDao.update(model));
-        });
+        data.getSlug().ifPresent(slug -> model.slug = slug);
+        data.getName().ifPresent(name -> model.name = name);
+        data.getCategory().ifPresent(category -> model.category = category);
+        data.getDescription().ifPresent(description -> model.description = description);
+        return fromModel(archiveDao.update(model));
     }
 
     private static Archive fromModel(ArchiveModel model) {

diff --git a/...-backends/judgels-server-app/src/main/java/judgels/jerahmeel/chapter/ChapterResource.java b/...-backends/judgels-server-app/src/main/java/judgels/jerahmeel/chapter/ChapterResource.java
@@ -52,8 +52,9 @@ public Chapter createChapter(AuthHeader authHeader, ChapterCreateData data) {
     @UnitOfWork
     public Chapter updateChapter(AuthHeader authHeader, String chapterJid, ChapterUpdateData data) {
         String actorJid = actorChecker.check(authHeader);
+        checkFound(chapterStore.getChapterByJid(chapterJid));
         checkAllowed(roleChecker.isAdmin(actorJid));
 
-        return checkFound(chapterStore.updateChapter(chapterJid, data));
+        return chapterStore.updateChapter(chapterJid, data);
     }
 }
diff --git a/...els-backends/judgels-server-app/src/main/java/judgels/jerahmeel/chapter/ChapterStore.java b/...els-backends/judgels-server-app/src/main/java/judgels/jerahmeel/chapter/ChapterStore.java
@@ -95,12 +95,10 @@ public Chapter createChapter(ChapterCreateData data) {
         return fromModel(chapterDao.insert(model));
     }
 
-
-    public Optional<Chapter> updateChapter(String chapterJid, ChapterUpdateData data) {
-        return chapterDao.selectByJid(chapterJid).map(model -> {
-            data.getName().ifPresent(name -> model.name = name);
-            return fromModel(chapterDao.update(model));
-        });
+    public Chapter updateChapter(String chapterJid, ChapterUpdateData data) {
+        ChapterModel model = chapterDao.findByJid(chapterJid);
+        data.getName().ifPresent(name -> model.name = name);
+        return fromModel(chapterDao.update(model));
     }
 
     private static Chapter fromModel(ChapterModel model) {

diff --git a/...ls-backends/judgels-server-app/src/main/java/judgels/jerahmeel/course/CourseResource.java b/...ls-backends/judgels-server-app/src/main/java/judgels/jerahmeel/course/CourseResource.java
@@ -82,8 +82,9 @@ public Course createCourse(AuthHeader authHeader, CourseCreateData data) {
     @UnitOfWork
     public Course updateCourse(AuthHeader authHeader, String courseJid, CourseUpdateData data) {
         String actorJid = actorChecker.check(authHeader);
+        checkFound(courseStore.getCourseByJid(courseJid));
         checkAllowed(roleChecker.isAdmin(actorJid));
 
-        return checkFound(courseStore.updateCourse(courseJid, data));
+        return courseStore.updateCourse(courseJid, data);
     }
 }
diff --git a/judgels-backends/judgels-server-app/src/main/java/judgels/jerahmeel/course/CourseStore.java b/judgels-backends/judgels-server-app/src/main/java/judgels/jerahmeel/course/CourseStore.java
@@ -49,22 +49,21 @@ public Course createCourse(CourseCreateData data) {
         return fromModel(courseDao.insert(model));
     }
 
-    public Optional<Course> updateCourse(String courseJid, CourseUpdateData data) {
-        return courseDao.selectByJid(courseJid).map(model -> {
-            if (data.getSlug().isPresent()) {
-                String newSlug = data.getSlug().get();
-                if (model.slug == null || !model.slug.equals(newSlug)) {
-                    if (courseDao.selectBySlug(newSlug).isPresent()) {
-                        throw CourseErrors.slugAlreadyExists(newSlug);
-                    }
+    public Course updateCourse(String courseJid, CourseUpdateData data) {
+        CourseModel model = courseDao.findByJid(courseJid);
+        if (data.getSlug().isPresent()) {
+            String newSlug = data.getSlug().get();
+            if (model.slug == null || !model.slug.equals(newSlug)) {
+                if (courseDao.selectBySlug(newSlug).isPresent()) {
+                    throw CourseErrors.slugAlreadyExists(newSlug);
                 }
             }
+        }
 
-            data.getSlug().ifPresent(slug -> model.slug = slug);
-            data.getName().ifPresent(name -> model.name = name);
-            data.getDescription().ifPresent(description -> model.description = description);
-            return fromModel(courseDao.update(model));
-        });
+        data.getSlug().ifPresent(slug -> model.slug = slug);
+        data.getName().ifPresent(name -> model.name = name);
+        data.getDescription().ifPresent(description -> model.description = description);
+        return fromModel(courseDao.update(model));
     }
 
     private static Course fromModel(CourseModel model) {

diff --git a/...nds/judgels-server-app/src/main/java/judgels/jerahmeel/problemset/ProblemSetResource.java b/...nds/judgels-server-app/src/main/java/judgels/jerahmeel/problemset/ProblemSetResource.java
@@ -142,9 +142,10 @@ public ProblemSet createProblemSet(AuthHeader authHeader, ProblemSetCreateData d
     @UnitOfWork
     public ProblemSet updateProblemSet(AuthHeader authHeader, String problemSetJid, ProblemSetUpdateData data) {
         String actorJid = actorChecker.check(authHeader);
+        checkFound(problemSetStore.getProblemSetByJid(problemSetJid));
         checkAllowed(roleChecker.isAdmin(actorJid));
 
-        return checkFound(problemSetStore.updateProblemSet(problemSetJid, data));
+        return problemSetStore.updateProblemSet(problemSetJid, data);
     }
 
     @Override

diff --git a/...ckends/judgels-server-app/src/main/java/judgels/jerahmeel/problemset/ProblemSetStore.java b/...ckends/judgels-server-app/src/main/java/judgels/jerahmeel/problemset/ProblemSetStore.java
@@ -134,33 +134,32 @@ public ProblemSet createProblemSet(ProblemSetCreateData data) {
         return fromModel(problemSetDao.insert(model));
     }
 
-    public Optional<ProblemSet> updateProblemSet(String problemSetJid, ProblemSetUpdateData data) {
-        return problemSetDao.selectByJid(problemSetJid).map(model -> {
-            if (data.getSlug().isPresent()) {
-                String newSlug = data.getSlug().get();
-                if (model.slug == null || !model.slug.equals(newSlug)) {
-                    if (problemSetDao.selectBySlug(newSlug).isPresent()) {
-                        throw ProblemSetErrors.slugAlreadyExists(newSlug);
-                    }
+    public ProblemSet updateProblemSet(String problemSetJid, ProblemSetUpdateData data) {
+        ProblemSetModel model = problemSetDao.findByJid(problemSetJid);
+        if (data.getSlug().isPresent()) {
+            String newSlug = data.getSlug().get();
+            if (model.slug == null || !model.slug.equals(newSlug)) {
+                if (problemSetDao.selectBySlug(newSlug).isPresent()) {
+                    throw ProblemSetErrors.slugAlreadyExists(newSlug);
                 }
             }
+        }
 
-            if (data.getArchiveSlug().isPresent()) {
-                String newArchiveSlug = data.getArchiveSlug().get();
-                Optional<ArchiveModel> archiveModel = archiveDao.selectBySlug(newArchiveSlug);
-                if (!archiveModel.isPresent()) {
-                    throw ProblemSetErrors.archiveSlugNotFound(newArchiveSlug);
-                }
-                model.archiveJid = archiveModel.get().jid;
+        if (data.getArchiveSlug().isPresent()) {
+            String newArchiveSlug = data.getArchiveSlug().get();
+            Optional<ArchiveModel> archiveModel = archiveDao.selectBySlug(newArchiveSlug);
+            if (!archiveModel.isPresent()) {
+                throw ProblemSetErrors.archiveSlugNotFound(newArchiveSlug);
             }
+            model.archiveJid = archiveModel.get().jid;
+        }
 
 
-            data.getSlug().ifPresent(slug -> model.slug = slug);
-            data.getName().ifPresent(name -> model.name = name);
-            data.getDescription().ifPresent(description -> model.description = description);
-            data.getContestTime().ifPresent(contestTime -> model.contestTime = contestTime);
-            return fromModel(problemSetDao.update(model));
-        });
+        data.getSlug().ifPresent(slug -> model.slug = slug);
+        data.getName().ifPresent(name -> model.name = name);
+        data.getDescription().ifPresent(description -> model.description = description);
+        data.getContestTime().ifPresent(contestTime -> model.contestTime = contestTime);
+        return fromModel(problemSetDao.update(model));
     }
 
     private static ProblemSet fromModel(ProblemSetModel model) {

diff --git a/judgels-backends/judgels-server-app/src/main/java/judgels/jophiel/user/UserResource.java b/judgels-backends/judgels-server-app/src/main/java/judgels/jophiel/user/UserResource.java
@@ -157,7 +157,7 @@ public UsersUpsertResponse upsertUsers(AuthHeader authHeader, String csv) throws
                         .email(getCsvValue(headerMap, line, "email"))
                         .build();
 
-                user = userStore.updateUser(existingUser.get().getJid(), data).get();
+                user = userStore.updateUser(existingUser.get().getJid(), data);
                 updatedUsernames.add(username);
                 existingInfo = infoStore.getInfo(user.getJid());
             } else {

diff --git a/judgels-backends/judgels-server-app/src/main/java/judgels/jophiel/user/UserStore.java b/judgels-backends/judgels-server-app/src/main/java/judgels/jophiel/user/UserStore.java
@@ -82,40 +82,36 @@ public Page<User> getUsers(int pageNumber, int pageSize, Optional<String> orderB
                 .mapPage(p -> Lists.transform(p, UserStore::fromModel));
     }
 
-    public Optional<User> updateUser(String userJid, UserUpdateData data) {
-        return userDao.selectByJid(userJid).map(model -> {
-            toModel(data, model);
-            return fromModel(userDao.updateByJid(userJid, model));
-        });
+    public User updateUser(String userJid, UserUpdateData data) {
+        UserModel model = userDao.findByJid(userJid);
+        toModel(data, model);
+        return fromModel(userDao.updateByJid(userJid, model));
     }
 
     public void validateUserPassword(String userJid, String password) {
-        userDao.selectByJid(userJid).ifPresent(model -> {
-            if (!validatePassword(password, model.password)) {
-                throw new IllegalArgumentException();
-            }
-        });
+        UserModel model = userDao.findByJid(userJid);
+        if (!validatePassword(password, model.password)) {
+            throw new IllegalArgumentException();
+        }
     }
 
     public void updateUserPassword(String userJid, String newPassword) {
-        userDao.selectByJid(userJid).ifPresent(model -> {
-            updateUser(userJid, new UserUpdateData.Builder()
-                    .username(model.username)
-                    .password(newPassword)
-                    .build());
-        });
+        UserModel model = userDao.findByJid(userJid);
+        updateUser(userJid, new UserUpdateData.Builder()
+                .username(model.username)
+                .password(newPassword)
+                .build());
     }
 
     public Optional<String> getUserAvatarFilename(String userJid) {
         return userDao.selectByJid(userJid).flatMap(model ->
                 Optional.ofNullable(model.avatarFilename));
     }
 
-    public Optional<User> updateUserAvatar(String userJid, @Nullable String newAvatarFilename) {
-        return userDao.selectByJid(userJid).map(model -> {
-            model.avatarFilename = newAvatarFilename;
-            return fromModel(userDao.update(model));
-        });
+    public void updateUserAvatar(String userJid, @Nullable String newAvatarFilename) {
+        UserModel model = userDao.findByJid(userJid);
+        model.avatarFilename = newAvatarFilename;
+        userDao.update(model);
     }
 
     public Optional<String> translateUsernameToJid(String username) {

diff --git a/...ends/judgels-server-app/src/main/java/judgels/jophiel/user/avatar/UserAvatarResource.java b/...ends/judgels-server-app/src/main/java/judgels/jophiel/user/avatar/UserAvatarResource.java
@@ -59,7 +59,7 @@ public void deleteAvatar(AuthHeader authHeader, String userJid) {
         String actorJid = actorChecker.check(authHeader);
         checkAllowed(roleChecker.canManage(actorJid, userJid));
 
-        checkFound(userStore.updateUserAvatar(userJid, null));
+        userStore.updateUserAvatar(userJid, null);
     }
 
     @Override

diff --git a/judgels-backends/judgels-server-app/src/main/java/judgels/uriel/contest/ContestResource.java b/judgels-backends/judgels-server-app/src/main/java/judgels/uriel/contest/ContestResource.java
@@ -78,7 +78,7 @@ public Contest updateContest(AuthHeader authHeader, String contestJid, ContestUp
         Contest contest = checkFound(contestStore.getContestByJid(contestJid));
 
         checkAllowed(contestRoleChecker.canManage(actorJid, contest));
-        contest = checkFound(contestStore.updateContest(contestJid, data));
+        contest = contestStore.updateContest(contestJid, data);
 
         contestLogger.log(contestJid, "UPDATE_CONTEST");
 
@@ -194,7 +194,7 @@ public ContestDescription getContestDescription(Optional<AuthHeader> authHeader,
 
         contestLogger.log(contest.getJid(), "OPEN_CONTEST");
 
-        String description = checkFound(contestStore.getContestDescription(contest.getJid()));
+        String description = contestStore.getContestDescription(contest.getJid());
         return new ContestDescription.Builder()
                 .description(description)
                 .profilesMap(profileStore.parseProfiles(description))
@@ -212,8 +212,7 @@ public ContestDescription updateContestDescription(
         Contest contest = checkFound(contestStore.getContestByJid(contestJid));
         checkAllowed(contestRoleChecker.canManage(actorJid, contest));
 
-        ContestDescription newDescription =
-                checkFound(contestStore.updateContestDescription(contest.getJid(), description));
+        ContestDescription newDescription = contestStore.updateContestDescription(contest.getJid(), description);
 
         contestLogger.log(contest.getJid(), "UPDATE_DESCRIPTION");
 

diff --git a/judgels-backends/judgels-server-app/src/main/java/judgels/uriel/contest/ContestStore.java b/judgels-backends/judgels-server-app/src/main/java/judgels/uriel/contest/ContestStore.java
@@ -171,36 +171,34 @@ public Contest createContest(ContestCreateData contestCreateData) {
         return fromModel(contestDao.insert(model));
     }
 
-    public Optional<Contest> updateContest(String contestJid, ContestUpdateData contestUpdateData) {
-        return contestDao.selectByJid(contestJid).map(model -> {
-            if (contestUpdateData.getSlug().isPresent()) {
-                String newSlug = contestUpdateData.getSlug().get();
-                if (model.slug == null || !model.slug.equals(newSlug)) {
-                    if (contestDao.selectBySlug(newSlug).isPresent()) {
-                        throw ContestErrors.slugAlreadyExists(newSlug);
-                    }
+    public Contest updateContest(String contestJid, ContestUpdateData data) {
+        ContestModel model = contestDao.findByJid(contestJid);
+        if (data.getSlug().isPresent()) {
+            String newSlug = data.getSlug().get();
+            if (model.slug == null || !model.slug.equals(newSlug)) {
+                if (contestDao.selectBySlug(newSlug).isPresent()) {
+                    throw ContestErrors.slugAlreadyExists(newSlug);
                 }
             }
+        }
 
-            contestUpdateData.getSlug().ifPresent(slug -> model.slug = slug);
-            contestUpdateData.getName().ifPresent(name -> model.name = name);
-            contestUpdateData.getStyle().ifPresent(style -> model.style = style.name());
-            contestUpdateData.getBeginTime().ifPresent(time -> model.beginTime = time);
-            contestUpdateData.getDuration().ifPresent(duration -> model.duration = duration.toMillis());
-            return fromModel(contestDao.update(model));
-        });
+        data.getSlug().ifPresent(slug -> model.slug = slug);
+        data.getName().ifPresent(name -> model.name = name);
+        data.getStyle().ifPresent(style -> model.style = style.name());
+        data.getBeginTime().ifPresent(time -> model.beginTime = time);
+        data.getDuration().ifPresent(duration -> model.duration = duration.toMillis());
+        return fromModel(contestDao.update(model));
     }
 
-    public Optional<String> getContestDescription(String contestJid) {
-        return contestDao.selectByJid(contestJid).map(model -> model.description);
+    public String getContestDescription(String contestJid) {
+        return contestDao.findByJid(contestJid).description;
     }
 
-    public Optional<ContestDescription> updateContestDescription(String contestJid, ContestDescription description) {
-        return contestDao.selectByJid(contestJid).map(model -> {
-            model.description = description.getDescription();
-            contestDao.update(model);
-            return description;
-        });
+    public ContestDescription updateContestDescription(String contestJid, ContestDescription description) {
+        ContestModel model = contestDao.findByJid(contestJid);
+        model.description = description.getDescription();
+        contestDao.update(model);
+        return description;
     }
 
     public String importDump(ContestDump dump) {

diff --git a/...server-app/src/main/java/judgels/uriel/contest/announcement/ContestAnnouncementStore.java b/...server-app/src/main/java/judgels/uriel/contest/announcement/ContestAnnouncementStore.java
@@ -45,11 +45,8 @@ public Page<ContestAnnouncement> getAnnouncements(String contestJid, Optional<St
                 .mapPage(p -> Lists.transform(p, ContestAnnouncementStore::fromModel));
     }
 
-    public ContestAnnouncement updateAnnouncement(
-            String contestJid,
-            String announcementJid,
-            ContestAnnouncementData data) {
-        ContestAnnouncementModel model = announcementDao.selectByJid(announcementJid).get();
+    public ContestAnnouncement updateAnnouncement(String contestJid, String announcementJid, ContestAnnouncementData data) {
+        ContestAnnouncementModel model = announcementDao.findByJid(announcementJid);
         toModel(contestJid, data, model);
         return fromModel(announcementDao.update(model));
     }

diff --git a/...r-app/src/main/java/judgels/uriel/contest/clarification/ContestClarificationResource.java b/...r-app/src/main/java/judgels/uriel/contest/clarification/ContestClarificationResource.java
@@ -163,12 +163,13 @@ public ContestClarification answerClarification(
 
         String actorJid = actorChecker.check(authHeader);
         Contest contest = checkFound(contestStore.getContestByJid(contestJid));
+        checkFound(clarificationStore.getClarification(contestJid, clarificationJid));
         checkAllowed(clarificationRoleChecker.canManage(actorJid, contest));
 
-        ContestClarification clarification = checkFound(clarificationStore.answerClarification(
+        ContestClarification clarification = clarificationStore.answerClarification(
                 contestJid,
                 clarificationJid,
-                data.getAnswer()));
+                data.getAnswer());
 
         contestLogger.log(contestJid, "ANSWER_CLARIFICATION", clarificationJid, clarification.getTopicJid());