-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[fix] 이력서 등록 사이즈 제한, url 제한 문제 해결 #278
Conversation
기존 env에서 넘겨받는 방식에서 application.yml에서 넘겨 받도록 변경
String, String으로 key value를 사용하고 있는 상황이여서 StringRedisTemplate으로 변경
redis 연결 성공 or 실패 여부 확인 가능하도록 connection check 작성
upload file size 문제 해결을 위해 spring servlet multipart size 키움
WalkthroughThis pull request introduces several updates across the codebase. The resume PDF column length is increased, and the Redis configuration is aligned with Spring Boot conventions by replacing custom templates with Changes
Sequence Diagram(s)sequenceDiagram
participant A as User
participant B as ResumeController
participant C as S3Uploader
participant D as S3 Storage
participant E as Database
A->>B: POST /api/v1/resumes (multipart)
B->>C: uploadPdf(file)
C->>D: uploadToS3(file)
D-->>C: URL response
C-->>B: file URL
B->>E: save Resume (with file URL)
E-->>B: confirmation
B-->>A: 201 Created
sequenceDiagram
participant RC as RedisConnectionChecker
participant RCF as RedisConnectionFactory
participant R as Redis
participant L as Logger
RC->>RCF: getConnection()
RCF->>R: ping()
R-->>RCF: pong
RCF-->>RC: connection result
RC->>L: log info/error
Suggested Reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review by ChatGPT
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
backend/src/main/java/com/techeer/backend/global/redis/RedisConnectionChecker.java (1)
1-25
: LGTM! Good addition of Redis connection validation.The implementation of
RedisConnectionChecker
is a good practice that helps identify Redis connectivity issues early in the application lifecycle.Consider internationalizing log messages.
The log messages are currently in Korean. Consider using message properties for internationalization support.
- log.info("✅ Redis 연결 성공!"); - log.error("❌ Redis 연결 실패!", e); + log.info("✅ Redis connection successful!"); + log.error("❌ Redis connection failed!", e);backend/src/test/java/com/techeer/backend/api/resume/controller/ResumeServiceIntegrationTest.java (1)
76-170
: Consider uncommenting and implementing the additional validations.The test thoroughly validates the resume creation flow. However, there are commented validations for TechStack and Company that should be implemented to ensure complete coverage.
- // assertTrue(savedResume.getTechStackNames().contains("Java")); - // assertTrue(savedResume.getCompanyNames().contains("TechCompany")); + assertThat(savedResume.getTechStacks()) + .extracting("name") + .contains("Java", "Spring"); + assertThat(savedResume.getCompanies()) + .extracting("name") + .contains("TechCompany");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
backend/src/main/java/com/techeer/backend/api/resume/domain/ResumePdf.java
(1 hunks)backend/src/main/java/com/techeer/backend/global/config/RedisConfig.java
(2 hunks)backend/src/main/java/com/techeer/backend/global/redis/RedisConnectionChecker.java
(1 hunks)backend/src/main/java/com/techeer/backend/global/redis/RedisService.java
(2 hunks)backend/src/main/java/com/techeer/backend/infra/aws/S3Uploader.java
(1 hunks)backend/src/main/resources/application.yml
(1 hunks)backend/src/test/java/com/techeer/backend/api/resume/controller/ResumeServiceIntegrationTest.java
(1 hunks)backend/src/test/java/com/techeer/backend/api/resume/service/ResumeServiceIntegrationTest.java
(0 hunks)backend/src/test/resources/application.yml
(2 hunks)
💤 Files with no reviewable changes (1)
- backend/src/test/java/com/techeer/backend/api/resume/service/ResumeServiceIntegrationTest.java
✅ Files skipped from review due to trivial changes (1)
- backend/src/main/java/com/techeer/backend/infra/aws/S3Uploader.java
🔇 Additional comments (7)
backend/src/main/java/com/techeer/backend/global/redis/RedisService.java (1)
9-9
: LGTM! Good migration to StringRedisTemplate.The switch to
StringRedisTemplate
is appropriate as we're only dealing with string values. This change helps resolve the Redis bean duplication issue by using Spring Boot's standard template.Also applies to: 20-20, 23-23, 29-29, 35-35
backend/src/main/java/com/techeer/backend/global/config/RedisConfig.java (1)
13-13
: LGTM! Good standardization of Redis configuration.The changes properly align with Spring Boot conventions:
- Property keys now follow Spring Boot's standard naming
- Simplified Redis template configuration by using
StringRedisTemplate
Also applies to: 16-16, 19-19, 32-33
backend/src/main/java/com/techeer/backend/api/resume/domain/ResumePdf.java (1)
42-42
:❓ Verification inconclusive
LGTM! URL length limit increased appropriately.
The increase of the PDF URL column length to 1000 characters should resolve the URL size exceedance issue.
Consider monitoring URL lengths in production to ensure this limit remains sufficient:
🏁 Script executed:
#!/bin/bash # Check the length of PDF URLs in the database echo "SELECT LENGTH(resume_pdf_url) as url_length, COUNT(*) as count FROM resume_pdf GROUP BY LENGTH(resume_pdf_url) ORDER BY url_length DESC LIMIT 5;" | mysql -u your_user -p your_databaseLength of output: 291
LGTM! PDF URL column length increased to 1000.
The updated column length appears correct to address the S3 URL size limitation. However, our automated check for monitoring URL lengths did not run successfully (the mysql client was not available), so please verify manually in your production environment to confirm that URL lengths remain within expected bounds. For example, you might run:
echo "SELECT LENGTH(resume_pdf_url) as url_length, COUNT(*) as count FROM resume_pdf GROUP BY LENGTH(resume_pdf_url) ORDER BY url_length DESC LIMIT 5;" | mysql -u your_user -p your_databasebackend/src/test/java/com/techeer/backend/api/resume/controller/ResumeServiceIntegrationTest.java (1)
41-74
: LGTM! Well-structured test class setup.The test class is properly configured with necessary annotations and dependencies for integration testing.
backend/src/test/resources/application.yml (2)
2-5
: LGTM! Appropriate file size limits configured.The multipart configuration with 100MB file size and 200MB request size limits is reasonable for handling resume PDF uploads.
32-36
:❓ Verification inconclusive
Verify Redis environment variables in CI/CD pipeline.
Ensure that the Redis environment variables (REDIS_HOST, REDIS_PORT, REDIS_PASSWORD) are properly configured in your CI/CD pipeline for test environments.
🏁 Script executed:
#!/bin/bash # Check if Redis environment variables are set in CI/CD env | grep REDIS_Length of output: 19
Action Required: Verify CI/CD Redis Environment Variables
It appears that none of the Redis environment variables (REDIS_HOST, REDIS_PORT, REDIS_PASSWORD) were exposed in the CI/CD environment during the test check. Please manually confirm that these variables are correctly defined and available in the CI/CD pipeline configuration for the test environments.
- File: backend/src/test/resources/application.yml (Lines 32-36)
- Context: Ensure that the substitutions for
${REDIS_HOST}
,${REDIS_PORT}
, and${REDIS_PASSWORD}
will have valid values when tests run in the CI/CD environment.backend/src/main/resources/application.yml (1)
2-5
: LGTM! Production file size limits match test configuration.The multipart configuration is consistent with the test environment, ensuring uniform behavior across environments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ 확인했습니다.
@@ -39,7 +39,7 @@ public class ResumePdf { | |||
|
|||
@Embedded | |||
@AttributeOverrides({ | |||
@AttributeOverride(name = "pdfUrl", column = @Column(name = "resume_pdf_url")), | |||
@AttributeOverride(name = "pdfUrl", column = @Column(name = "resume_pdf_url", length = 1000)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JPA에서 @Embedded와 @embeddable 차이점 검색해보면 좋을 거 같아!
https://lordofkangs.tistory.com/380
간단히 말하면
@embeddable: 값 타입을 정의하는 곳
@Embedded: 값 타입을 사용하는 곳
} | ||
|
||
public void cacheRefreshToken(String refreshToken) { | ||
String key = "refreshToken:" + refreshToken; | ||
log.info("refresh token cache key: {}", key); | ||
// 리프레시 토큰을 Redis에 저장 (예: 7일 만료) | ||
redisTemplate.opsForValue().set(key, refreshToken, Duration.ofMillis(refreshTokenExpirationPeriod)); | ||
stringRedisTemplate.opsForValue().set(key, refreshToken, Duration.ofMillis(refreshTokenExpirationPeriod)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
리프레시 토큰을 보통 7일간 저장해?(진짜 모름)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
변경 사항
이력서 등록 시 발생하는 문제 해결 및 통합 테스트 작성
redis bean 2개 뜨는 문제로 인한 문제 해결

이력서 maximum upload file size 문제 해결

이력서 file url size 초과 문제 해결

테스트 결과 (테스트를 했으면)
Summary by CodeRabbit