Skip to content
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,12 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import life.mosu.mosuserver.domain.application.AdmissionTicketImageJpaEntity;
import life.mosu.mosuserver.domain.application.AdmissionTicketImageJpaRepository;
import life.mosu.mosuserver.domain.application.ApplicationJpaEntity;
import life.mosu.mosuserver.domain.application.ApplicationJpaRepository;
import life.mosu.mosuserver.domain.application.ApplicationSchoolJpaRepository;
import life.mosu.mosuserver.domain.applicationschool.ApplicationSchoolJpaEntity;
import life.mosu.mosuserver.domain.school.AddressJpaVO;
import life.mosu.mosuserver.domain.school.SchoolJpaEntity;
import life.mosu.mosuserver.domain.school.SchoolJpaRepository;
import life.mosu.mosuserver.global.exception.CustomRuntimeException;
Expand Down Expand Up @@ -38,38 +36,36 @@ public class ApplicationService {
// 신청
@Transactional
public ApplicationResponse apply(Long userId, ApplicationRequest request) {
Set<ApplicationSchoolRequest> schools = request.schools();
List<ApplicationSchoolJpaEntity> schoolEntities = new ArrayList<>();
Set<ApplicationSchoolRequest> schoolRequests = request.schools();
List<ApplicationSchoolJpaEntity> savedEntities = new ArrayList<>();

Set<Long> schoolIds = schools.stream()
.map(ApplicationSchoolRequest::schoolId)
.collect(Collectors.toSet());

if (applicationSchoolJpaRepository.existsByUserIdAndSchoolIdIn(userId, schoolIds)) {
throw new CustomRuntimeException(ErrorCode.APPLICATION_SCHOOL_ALREADY_APPLIED);
}

ApplicationJpaEntity application = request.toEntity(userId);
ApplicationJpaEntity applicationJpaEntity = applicationJpaRepository.save(application);
Long applicationId = applicationJpaEntity.getId();
ApplicationJpaEntity application = applicationJpaRepository.save(request.toEntity(userId));
Long applicationId = application.getId();

admissionTicketImageJpaRepository.save(
createAdmissionTicketImageIfPresent(request.admissionTicket(), applicationId));

schools.forEach(applicationSchoolRequest -> {
SchoolJpaEntity school = schoolJpaRepository.findById(
applicationSchoolRequest.schoolId())
for (ApplicationSchoolRequest schoolRequest : schoolRequests) {
Long schoolId = schoolJpaRepository.findBySchoolNameAndAreaAndExamDate(
schoolRequest.schoolName(),
schoolRequest.validatedArea(schoolRequest.area()),
schoolRequest.examDate())
.orElseThrow(() -> new CustomRuntimeException(ErrorCode.SCHOOL_NOT_FOUND))
Comment on lines +49 to +53

Choose a reason for hiding this comment

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

medium

It looks like you're querying the schoolJpaRepository to find a school by name, area, and exam date, and then immediately extracting the ID. Consider creating a method that directly retrieves the school ID based on these criteria to avoid fetching the entire entity when only the ID is needed. This could improve performance, especially if the SchoolJpaEntity contains a lot of data.

            Long schoolId = schoolJpaRepository.findSchoolIdBySchoolNameAndAreaAndExamDate(
                            schoolRequest.schoolName(),
                            schoolRequest.validatedArea(schoolRequest.area()),
                            schoolRequest.examDate())
                    .orElseThrow(() -> new CustomRuntimeException(ErrorCode.SCHOOL_NOT_FOUND));

.getId();

if (applicationSchoolJpaRepository.existsByUserIdAndSchoolId(userId, schoolId)) {
throw new CustomRuntimeException(ErrorCode.APPLICATION_SCHOOL_ALREADY_APPLIED);
}
Comment on lines +56 to +58

Choose a reason for hiding this comment

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

medium

The check for existing applications is performed after retrieving the schoolId. If findBySchoolNameAndAreaAndExamDate fails, the application will throw a SCHOOL_NOT_FOUND exception. It might be more efficient to check for existing applications before attempting to retrieve the schoolId. This could prevent unnecessary database queries when the school doesn't exist.

            if (applicationSchoolJpaRepository.existsByUserIdAndSchoolId(
                userId, schoolRequest.schoolName(), schoolRequest.area(), schoolRequest.examDate())) {
                throw new CustomRuntimeException(ErrorCode.APPLICATION_SCHOOL_ALREADY_APPLIED);
            }


SchoolJpaEntity school = schoolJpaRepository.findById(schoolId)
.orElseThrow(() -> new CustomRuntimeException(ErrorCode.SCHOOL_NOT_FOUND));

AddressJpaVO address = school.getAddress();
ApplicationSchoolJpaEntity applicationSchoolJpaEntity = applicationSchoolRequest.toEntity(
userId, applicationId, address);
ApplicationSchoolJpaEntity saved = applicationSchoolJpaRepository.save(
applicationSchoolJpaEntity);
schoolEntities.add(saved);
});
ApplicationSchoolJpaEntity applicationSchool = schoolRequest.toEntity(userId,
applicationId, school);
savedEntities.add(applicationSchoolJpaRepository.save(applicationSchool));
}
Comment on lines +48 to +66
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Optimize database calls and improve error handling.

The refactored logic has several issues that impact performance and maintainability:

  1. Redundant database call: Lines 60-61 fetch the school entity again after already retrieving it in lines 49-54
  2. Performance concern: The individual processing approach makes multiple database calls per school instead of bulk operations
  3. Potential transaction rollback: If any school fails validation, the entire application (including previously processed schools) will be rolled back

Apply this optimization to eliminate the redundant database call:

        for (ApplicationSchoolRequest schoolRequest : schoolRequests) {
-            Long schoolId = schoolJpaRepository.findBySchoolNameAndAreaAndExamDate(
+            SchoolJpaEntity school = schoolJpaRepository.findBySchoolNameAndAreaAndExamDate(
                            schoolRequest.schoolName(),
                            schoolRequest.validatedArea(schoolRequest.area()),
                            schoolRequest.examDate())
                    .orElseThrow(() -> new CustomRuntimeException(ErrorCode.SCHOOL_NOT_FOUND))
-                    .getId();
+                    ;

-            if (applicationSchoolJpaRepository.existsByUserIdAndSchoolId(userId, schoolId)) {
+            if (applicationSchoolJpaRepository.existsByUserIdAndSchoolId(userId, school.getId())) {
                throw new CustomRuntimeException(ErrorCode.APPLICATION_SCHOOL_ALREADY_APPLIED);
            }

-            SchoolJpaEntity school = schoolJpaRepository.findById(schoolId)
-                    .orElseThrow(() -> new CustomRuntimeException(ErrorCode.SCHOOL_NOT_FOUND));

            ApplicationSchoolJpaEntity applicationSchool = schoolRequest.toEntity(userId,
                    applicationId, school);
            savedEntities.add(applicationSchoolJpaRepository.save(applicationSchool));
        }

Run this script to verify the school lookup method exists and returns the expected type:


🏁 Script executed:

#!/bin/bash
# Description: Verify the school repository method signature and return type

# Search for the findBySchoolNameAndAreaAndExamDate method definition
ast-grep --pattern 'findBySchoolNameAndAreaAndExamDate($$$) {
  $$$
}'

# Also search for the method signature in interface definitions
rg -A 3 "findBySchoolNameAndAreaAndExamDate"

Length of output: 1077


Optimize ApplicationService school lookup and eliminate redundant call

Verified that findBySchoolNameAndAreaAndExamDate(...) already returns a SchoolJpaEntity, so you can reuse it instead of querying by ID twice.

• File: src/main/java/life/mosu/mosuserver/application/application/ApplicationService.java

  • Remove the extra findById(schoolId) lookup
  • Use the initially retrieved SchoolJpaEntity for both the existence check and entity creation
         for (ApplicationSchoolRequest schoolRequest : schoolRequests) {
-            Long schoolId = schoolJpaRepository.findBySchoolNameAndAreaAndExamDate(
+            SchoolJpaEntity school = schoolJpaRepository.findBySchoolNameAndAreaAndExamDate(
                             schoolRequest.schoolName(),
                             schoolRequest.validatedArea(schoolRequest.area()),
                             schoolRequest.examDate())
                     .orElseThrow(() -> new CustomRuntimeException(ErrorCode.SCHOOL_NOT_FOUND))
-                    .getId();
+
             if (applicationSchoolJpaRepository.existsByUserIdAndSchoolId(userId, school.getId())) {
                 throw new CustomRuntimeException(ErrorCode.APPLICATION_SCHOOL_ALREADY_APPLIED);
             }

-            SchoolJpaEntity school = schoolJpaRepository.findById(schoolId)
-                    .orElseThrow(() -> new CustomRuntimeException(ErrorCode.SCHOOL_NOT_FOUND));
 
             ApplicationSchoolJpaEntity applicationSchool = schoolRequest.toEntity(userId,
                     applicationId, school);
             savedEntities.add(applicationSchoolJpaRepository.save(applicationSchool));
         }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/application/application/ApplicationService.java
around lines 48 to 66, the code redundantly queries the school entity twice:
once with findBySchoolNameAndAreaAndExamDate and again with findById. To fix
this, store the SchoolJpaEntity returned by findBySchoolNameAndAreaAndExamDate
and reuse it for the existence check and entity creation, removing the second
findById call entirely.


return ApplicationResponse.of(applicationId, schoolEntities);
return ApplicationResponse.of(applicationId, savedEntities);
}


Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
package life.mosu.mosuserver.domain.application;

import java.util.Collection;
import java.util.List;
import life.mosu.mosuserver.domain.applicationschool.ApplicationSchoolJpaEntity;
import org.springframework.data.jpa.repository.JpaRepository;

public interface ApplicationSchoolJpaRepository extends
JpaRepository<ApplicationSchoolJpaEntity, Long> {


boolean existsByUserIdAndSchoolIdIn(Long userId, Collection<Long> schoolIds);
boolean existsByUserIdAndSchoolId(Long userId, Long schoolId);
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Consider performance implications of individual existence checks.

The change from existsByUserIdAndSchoolIdIn to existsByUserIdAndSchoolId means the service layer will now make multiple individual database queries instead of one bulk query. This could impact performance when processing applications with multiple schools.

Consider:

  1. The performance impact of N individual queries vs 1 bulk query
  2. Whether the service layer should batch these checks
  3. Potential race conditions between the existence check and application creation

🏁 Script executed:

#!/bin/bash
# Check how this method is being used in the service layer
rg -A 5 -B 5 "existsByUserIdAndSchoolId" --type java

Length of output: 2979


Batch existence checks and guard against race conditions

Switching from a single bulk query to per-school existsByUserIdAndSchoolId calls in ApplicationService will execute one SELECT per school in the request. This may degrade performance for large batches and still leaves a window for duplicate inserts under concurrency. Please address the following:

• In ApplicationService.java (around the loop invoking existsByUserIdAndSchoolId), replace N individual queries with a single existsByUserIdAndSchoolIdIn(List<Long> schoolIds) call or pre-fetch all existing (userId, schoolId) pairs and check in memory.
• Add a unique constraint on (user_id, school_id) at the database level and catch the resulting exception (e.g. DataIntegrityViolationException) to handle any race-condition insert attempts.
• Ensure the check-then-insert logic runs within a single transaction or adjust isolation levels so that concurrent threads cannot slip past the existence check.

🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/domain/application/ApplicationSchoolJpaRepository.java
at line 10, the method existsByUserIdAndSchoolId causes multiple queries for
batch existence checks, which harms performance and allows race conditions.
Replace this with a method like existsByUserIdAndSchoolIdIn(List<Long>
schoolIds) to perform a single bulk query or pre-fetch all existing pairs and
check in memory. Additionally, add a unique constraint on (user_id, school_id)
in the database schema and handle DataIntegrityViolationException during inserts
to manage race conditions. Finally, ensure the check-and-insert logic runs
within a single transaction or adjust isolation levels to prevent concurrent
insert conflicts.


List<ApplicationSchoolJpaEntity> findAllByApplicationId(Long applicationId);

Expand Down
3 changes: 1 addition & 2 deletions src/main/java/life/mosu/mosuserver/domain/school/Area.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
public enum Area {
DAECHI("대치"),
MOKDONG("목동"),
DAEGU("대구");

NOWON("노원");
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

❓ Verification inconclusive

Verify data migration for the Area enum change.

The change from DAEGU to NOWON could break existing data if there are schools or applications that reference the DAEGU area. Ensure that:

  1. All existing data using DAEGU has been properly migrated
  2. No foreign key constraints will be violated
  3. This change is coordinated with any database schema updates

🏁 Script executed:

#!/bin/bash
# Check for any references to DAEGU in the codebase
rg -i "DAEGU" --type java

# Check for any database migration files
fd -e sql -e xml migration

Length of output: 54


Add a database migration for the Area enum change

Your enum update replaces DAEGU with NOWON but there are no existing migration scripts to handle this. Without one, any stored records with DAEGU will fail to deserialize. Please:

  • Add a migration (e.g. Flyway/Liquibase) that
    1. Updates all rows in the schools (or related) table: UPDATE … SET area = 'NOWON' WHERE area = 'DAEGU';
    2. (Optional) Protect against future failures by handling unknown enum values in your deserializer.
  • Verify there are no stray references to DAEGU in any file type (YAML, SQL, XML, etc.) beyond Java sources.
  • Coordinate this change with your release process so production data remains consistent.
🤖 Prompt for AI Agents
In src/main/java/life/mosu/mosuserver/domain/school/Area.java at line 6, the
enum value DAEGU was replaced with NOWON without a corresponding database
migration. To fix this, create a migration script (using Flyway or Liquibase)
that updates all existing records in the relevant table by running an SQL update
statement to replace 'DAEGU' with 'NOWON'. Additionally, review all project
files for any remaining references to 'DAEGU' and remove or update them.
Optionally, enhance the enum deserialization logic to handle unknown values
gracefully to prevent future failures. Coordinate this migration with your
deployment to ensure data consistency in production.

private final String areaName;

Area(String areaName) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
package life.mosu.mosuserver.domain.school;

import java.time.LocalDate;
import java.util.Optional;
import org.springframework.data.jpa.repository.JpaRepository;

public interface SchoolJpaRepository extends JpaRepository<SchoolJpaEntity, Long> {

Optional<SchoolJpaEntity> findBySchoolNameAndAreaAndExamDate(String schoolName, Area area,
LocalDate examDate);
Comment on lines +9 to +10
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Consider database indexing for the new query method.

The new findBySchoolNameAndAreaAndExamDate method performs a composite query that may have performance implications. Ensure that:

  1. A composite database index exists on (school_name, area, exam_date) columns
  2. The query performance is acceptable for the expected data volume
  3. Consider if this method should have caching if it's called frequently

🏁 Script executed:

#!/bin/bash
# Check for database migration files that might contain index creation
rg -A 10 -B 10 "school_name.*area.*exam_date|exam_date.*area.*school_name" --type sql

# Look for any existing index definitions
rg -i "index.*school" --type sql

Length of output: 420


#!/bin/bash
# Locate the SchoolJpaEntity to inspect JPA @Table index definitions
entity_file=$(find src -type f -name "SchoolJpaEntity.java" | head -n 1)
if [[ -n "$entity_file" ]]; then
  echo "Inspecting entity file: $entity_file"
  rg -A3 "@Table" -n "$entity_file"
  rg -i "@Index" -n "$entity_file"
else
  echo "SchoolJpaEntity.java not found in src/"
fi

# List database migration/changelog files
echo
echo "Migration files under src/main/resources/db/:"
find src/main/resources/db/ -type f | grep -E "\.(sql|xml|yml|yaml)$" || echo "  (none found)"

# Search migrations for index creation statements
echo
echo "Searching for explicit CREATE INDEX statements in migrations:"
rg -i "create\s\+index" -n src/main/resources/db/ || echo "  (no CREATE INDEX found)"

🏁 Script executed:

#!/bin/bash
# Locate the JPA entity to check @Table/@Index annotations
entity_file=$(find src -type f -name "SchoolJpaEntity.java" | head -n 1)
if [[ -n "$entity_file" ]]; then
  echo ">>> Inspecting $entity_file for table/index annotations"
  rg -A3 "@Table" -n "$entity_file" || echo "  (no @Table annotation found)"
  rg -n "@Index" -n "$entity_file" || echo "  (no @Index annotation found)"
else
  echo "ERROR: SchoolJpaEntity.java not found under src/"
fi

echo
# List all migration/changelog files in resources
echo ">>> Listing migration files under src/main/resources"
find src/main/resources -type f | grep -E "\.(sql|xml|yml|yaml)$" || echo "  (no migration files found)"

echo
# Search migrations for explicit CREATE INDEX statements
echo ">>> Searching migrations for CREATE INDEX"
rg -i "create\s+index" -n src/main/resources || echo "  (no CREATE INDEX statements found)"

Length of output: 1396


Add composite database index for the new query
The findBySchoolNameAndAreaAndExamDate lookup will trigger a full-table scan unless you introduce an index on (school_name, area, exam_date). I didn’t find any existing JPA @Index on SchoolJpaEntity or a migration that creates this composite index. Please:

  • In src/main/java/life/mosu/mosuserver/domain/school/SchoolJpaEntity.java, update the @Table annotation:
    @Table(
      name = "school",
      indexes = @Index(
        name = "idx_school_name_area_exam_date",
        columnList = "school_name,area,exam_date"
      )
    )
  • Or add a database migration (Flyway/Liquibase/SQL) to create:
    CREATE INDEX idx_school_name_area_exam_date
      ON school (school_name, area, exam_date);
  • Optionally, if this method is called very frequently, consider adding a second-level cache (e.g., Spring Cache) to reduce database hits.
🤖 Prompt for AI Agents
In src/main/java/life/mosu/mosuserver/domain/school/SchoolJpaEntity.java around
lines 9-10, the new query method findBySchoolNameAndAreaAndExamDate lacks a
composite index on (school_name, area, exam_date), causing full-table scans. Fix
this by updating the @Table annotation on SchoolJpaEntity to include an @Index
with name "idx_school_name_area_exam_date" and columnList
"school_name,area,exam_date". Alternatively, add a database migration script to
create this index. Optionally, consider adding a second-level cache if this
query is very frequent.

}
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,12 @@
import life.mosu.mosuserver.domain.application.Lunch;
import life.mosu.mosuserver.domain.application.Subject;
import life.mosu.mosuserver.domain.applicationschool.ApplicationSchoolJpaEntity;
import life.mosu.mosuserver.domain.school.AddressJpaVO;
import life.mosu.mosuserver.domain.school.Area;
import life.mosu.mosuserver.domain.school.SchoolJpaEntity;
import life.mosu.mosuserver.global.exception.CustomRuntimeException;
import life.mosu.mosuserver.global.exception.ErrorCode;

public record ApplicationSchoolRequest(
@NotNull(message = "학교 ID는 필수입니다.")
Long schoolId,

@NotBlank(message = "학교 이름은 필수입니다.")
String schoolName,
Expand All @@ -32,14 +30,14 @@ public record ApplicationSchoolRequest(
) {

public ApplicationSchoolJpaEntity toEntity(Long userId, Long applicationId,
AddressJpaVO address) {
SchoolJpaEntity school) {
return ApplicationSchoolJpaEntity.builder()
.userId(userId)
.applicationId(applicationId)
.schoolId(schoolId)
.schoolName(schoolName)
.area(validatedArea(area))
.address(address)
.schoolId(school.getId())
.schoolName(school.getSchoolName())
.area(school.getArea())
.address(school.getAddress())
.examDate(examDate)
.lunch(validatedLunch(lunch))
.subjects(validatedSubjects(subjects))
Expand All @@ -64,7 +62,7 @@ private Lunch validatedLunch(String lunch) {
}
}

private Area validatedArea(String area) {
public Area validatedArea(String area) {
try {
return Area.valueOf(area.toUpperCase());
} catch (IllegalArgumentException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,32 +3,33 @@
import java.time.LocalDate;
import java.util.Set;
import java.util.stream.Collectors;
import life.mosu.mosuserver.domain.application.Lunch;
import life.mosu.mosuserver.domain.application.Subject;
import life.mosu.mosuserver.domain.applicationschool.ApplicationSchoolJpaEntity;
import life.mosu.mosuserver.domain.school.Area;

public record ApplicationSchoolResponse(
Long applicationSchoolId,
Area area,
String area,
LocalDate examDate,
String schoolName,
Lunch lunch,
String lunch,
String examinationNumber,
Set<String> subjects
) {

public static ApplicationSchoolResponse from(ApplicationSchoolJpaEntity applicationSchool) {
String areaName = applicationSchool.getArea().getAreaName();
String lunchName = applicationSchool.getLunch().getLunchName();

Set<String> subjectNames = applicationSchool.getSubjects().stream()
.map(Subject::getSubjectName)
.collect(Collectors.toSet());

return new ApplicationSchoolResponse(
applicationSchool.getId(),
applicationSchool.getArea(),
areaName,
applicationSchool.getExamDate(),
applicationSchool.getSchoolName(),
applicationSchool.getLunch(),
lunchName,
applicationSchool.getExaminationNumber(),
subjectNames
);
Expand Down