[Feature] 이미지 업로드를 위한 S3 연동 #68
Conversation
📝 WalkthroughWalkthroughAdds an S3 image-upload subsystem: Gradle version/library updates; new AWS client, configuration, and properties; S3 helpers producing presigned URLs; per-user rate limiting (Bucket4j + Caffeine); DTOs and error types; facade/controller wiring to return upload URLs; aws.yml enabled. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as BloomingController
participant Facade as BloomingFacade
participant Repo as BloomingRepository
participant ImageCaller as ImageS3Caller (ImageS3Processor)
participant RateLimiter as PresignedUrlRateLimiter
participant AwsClient as AwsS3Client
participant AWS as S3Presigner/S3Client
Client->>Controller: POST /blooming (NewBlooming)
Controller->>Facade: uploadBloomingStatus(newBlooming)
Facade->>Repo: persist NewBlooming -> Blooming (spotId)
Facade->>ImageCaller: createUploadUrl(userId, ImagePrefix.FLOWERSPOT, spotId)
ImageCaller->>RateLimiter: consume(userId)
alt token available
RateLimiter-->>ImageCaller: ok
ImageCaller->>AwsClient: generateUploadUrl(bucket, path, filename, ttl)
AwsClient->>AWS: presign PutObjectRequest
AWS-->>AwsClient: presignedUrl
AwsClient-->>ImageCaller: S3ImageUrl(presignedUrl, imageUrl)
ImageCaller-->>Facade: BloomingImageUploadUrl.from(S3ImageUrl)
Facade-->>Controller: BloomingImageUploadUrl
Controller-->>Client: 200 { uploadUrl, imageUrl }
else rate limit exceeded
RateLimiter-->>ImageCaller: throws EXCEED_RATE_LIMIT (429)
ImageCaller-->>Facade: propagate error
Facade-->>Controller: propagate error
Controller-->>Client: 429 TOO_MANY_REQUESTS
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (1)pida-core/core-domain/src/main/kotlin/com/pida/flowerspot/FlowerSpotDetails.kt (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (5)
pida-clients/aws-client/src/main/kotlin/com/pida/client/aws/config/AwsConfig.kt (1)
36-47: Inject thecredentialProviderbean instead of calling the method directly.Calling
credentialProvider()directly creates a new instance each time instead of reusing the Spring-managed bean. This results in multiple credential provider instances being created unnecessarily.🔎 Proposed fix
- @Bean(destroyMethod = "close") // 스프링 종료 시 커넥션 풀 정리 - fun s3Client(): S3Client { + @Bean(destroyMethod = "close") + fun s3Client(credentialsProvider: AwsCredentialsProvider): S3Client { val client = S3Client .builder() - .credentialsProvider(credentialProvider()) + .credentialsProvider(credentialsProvider) .region(Region.of(awsProperties.region)) awsProperties.endpoint?.let { client.endpointOverride(URI.create(awsProperties.endpoint)) } return client.build() }Apply the same pattern to
s3Presigner():@Bean(destroyMethod = "close") - fun s3Presigner(): S3Presigner { + fun s3Presigner(credentialsProvider: AwsCredentialsProvider): S3Presigner { val client = S3Presigner .builder() - .credentialsProvider(credentialProvider()) + .credentialsProvider(credentialsProvider) .region(Region.of(awsProperties.region))pida-core/core-domain/src/main/kotlin/com/pida/support/aws/PresignedUrlRateLimitPolicy.kt (1)
6-13: Consider making the rate limit configurable.The hardcoded 5 requests per minute limit works for now, but making this configurable via properties would allow tuning without code changes.
🔎 Example using configuration properties
@ConfigurationProperties(prefix = "rate-limit.presigned-url") data class PresignedUrlRateLimitProperties( val capacity: Long = 5, val refillPerMinute: Long = 5, )Then inject and use these properties in a component that builds the
Bandwidth.pida-clients/aws-client/src/main/kotlin/com/pida/client/aws/image/ImageFileConstructor.kt (1)
32-35: Increase filename entropy by including numbers.The random filename generator uses only lowercase letters (26 possible characters), which provides less collision resistance than including numbers (36 possible characters). With only letters, the chance of collision is higher, especially at scale.
🔎 Proposed improvement
companion object { private val RANDOM = SecureRandom() // 파일명 생성용 난수기 - private const val LOWERCASE_A = 97 - private const val LOWERCASE_Z = 122 + private const val ALPHANUMERIC_CHARS = "abcdefghijklmnopqrstuvwxyz0123456789" private const val FILENAME_LENGTH = 24 } private fun randomFileName(): String = - (1..FILENAME_LENGTH) - .map { RANDOM.nextInt(LOWERCASE_A, LOWERCASE_Z + 1).toChar() } - .joinToString("") + (1..FILENAME_LENGTH) + .map { ALPHANUMERIC_CHARS[RANDOM.nextInt(ALPHANUMERIC_CHARS.length)] } + .joinToString("")pida-core/core-domain/src/main/kotlin/com/pida/support/aws/PresignedUrlRateLimiter.kt (1)
27-28: Unused helper method with TODO.The
retryAfterSecondsmethod is currently unused. The TODO comment indicates it may be used for Retry-After headers in the future.Would you like me to help implement the Retry-After header support now, or track this as a separate issue?
pida-clients/aws-client/src/main/kotlin/com/pida/client/aws/s3/AwsS3Client.kt (1)
17-19: Environment variable read at class-load time may not reflect runtime changes.The
profileis read fromSPRING_PROFILES_ACTIVEwhen the class is loaded (companion object initialization). If the environment variable changes or Spring's active profile differs from the OS environment variable, this could cause path mismatches.Consider injecting the active profile from Spring's
Environmentbean instead:🔎 Proposed refactor
+import org.springframework.core.env.Environment + @Component class AwsS3Client( private val s3Presigner: S3Presigner, private val s3Client: S3Client, + private val environment: Environment, ) { - companion object { - private val profile = System.getenv("SPRING_PROFILES_ACTIVE") ?: "dev" - } + private val profile: String + get() = environment.activeProfiles.firstOrNull() ?: "dev"This ensures the profile reflects Spring's actual runtime configuration.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
gradle/libs.versions.tomlpida-clients/aws-client/build.gradle.ktspida-clients/aws-client/src/main/kotlin/com/pida/client/aws/.gitkeeppida-clients/aws-client/src/main/kotlin/com/pida/client/aws/config/AwsConfig.ktpida-clients/aws-client/src/main/kotlin/com/pida/client/aws/config/AwsProperties.ktpida-clients/aws-client/src/main/kotlin/com/pida/client/aws/image/ImageFileConstructor.ktpida-clients/aws-client/src/main/kotlin/com/pida/client/aws/image/ImageS3Processor.ktpida-clients/aws-client/src/main/kotlin/com/pida/client/aws/s3/AwsS3Client.ktpida-clients/aws-client/src/main/resources/aws.ymlpida-core/core-api/src/main/kotlin/com/pida/presentation/v1/blooming/BloomingController.ktpida-core/core-api/src/main/resources/application.ymlpida-core/core-domain/build.gradle.ktspida-core/core-domain/src/main/kotlin/com/pida/blooming/BloomingFacade.ktpida-core/core-domain/src/main/kotlin/com/pida/blooming/BloomingImageUploadUrl.ktpida-core/core-domain/src/main/kotlin/com/pida/support/aws/ImagePrefix.ktpida-core/core-domain/src/main/kotlin/com/pida/support/aws/ImageS3Caller.ktpida-core/core-domain/src/main/kotlin/com/pida/support/aws/PresignedUrlRateLimitPolicy.ktpida-core/core-domain/src/main/kotlin/com/pida/support/aws/PresignedUrlRateLimiter.ktpida-core/core-domain/src/main/kotlin/com/pida/support/aws/S3ImageUrl.ktpida-core/core-domain/src/main/kotlin/com/pida/support/error/ErrorKind.ktpida-core/core-domain/src/main/kotlin/com/pida/support/error/ErrorType.kt
💤 Files with no reviewable changes (1)
- pida-clients/aws-client/build.gradle.kts
🧰 Additional context used
🧬 Code graph analysis (2)
pida-core/core-domain/build.gradle.kts (1)
build.gradle.kts (2)
implementation(43-56)libs(23-83)
pida-clients/aws-client/src/main/kotlin/com/pida/client/aws/config/AwsProperties.kt (2)
pida-clients/notification/src/main/kotlin/com/pida/client/notification/FcmConfig.kt (1)
fcmProperties(10-28)pida-clients/notification/src/main/kotlin/com/pida/client/notification/FcmProperties.kt (1)
prefix(5-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CI (21, 2.1.0, corretto)
🔇 Additional comments (11)
pida-core/core-api/src/main/resources/application.yml (1)
21-21: LGTM! The addition ofaws.ymlto the configuration imports is correct and all prerequisites are met.The
aws.ymlfile exists in thepida-clients/aws-clientmodule (which is a declared dependency of core-api), making it accessible via the classpath. AWS credentials are properly configured using environment variables (${AWS_ACCESS_KEY},${AWS_SECRET_KEY}, etc.) with fallback defaults for the local profile, and the configuration is profile-aware for local, dev, and prod environments.pida-core/core-domain/src/main/kotlin/com/pida/support/error/ErrorType.kt (1)
56-58: LGTM!The rate limit error type is correctly implemented with HTTP 429 status code, appropriate
TOO_MANY_REQUESTSkind, andWARNlevel. This aligns with standard HTTP semantics and the existing error handling patterns in the codebase.pida-core/core-domain/src/main/kotlin/com/pida/support/aws/S3ImageUrl.kt (1)
1-6: LGTM!Clean DTO design that appropriately separates the presigned upload URL from the final image URL. This separation is useful since clients need the presigned URL for upload but may want to store or display the permanent image URL.
pida-core/core-domain/src/main/kotlin/com/pida/support/error/ErrorKind.kt (1)
12-12: LGTM!The new
TOO_MANY_REQUESTSenum constant correctly represents HTTP 429 rate limiting scenarios and is properly referenced byEXCEED_RATE_LIMITinErrorType.pida-clients/aws-client/src/main/kotlin/com/pida/client/aws/config/AwsProperties.kt (1)
13-16: Verify secrets are not committed to version control.The credentials are correctly modeled for Spring Boot configuration binding. Ensure that actual
accessKeyandsecretKeyvalues are provided via environment variables, secrets manager, or profile-specific configuration that is excluded from version control (e.g., via.gitignore).pida-core/core-domain/src/main/kotlin/com/pida/support/aws/ImagePrefix.kt (1)
1-7: LGTM!Clean enum design that provides type safety for S3 path prefixes. The lowercase value is appropriate for S3 key naming conventions.
pida-core/core-domain/build.gradle.kts (1)
17-19: LGTM — Bucket4j is a solid choice for rate limiting.Bucket4j provides a robust token bucket implementation. The PR description mentions potential future migration to Redis for distributed rate limiting, which Bucket4j supports natively via its Redis integrations (Lettuce, Jedis, or Redisson with ProxyManager).
pida-core/core-domain/src/main/kotlin/com/pida/support/aws/ImageS3Caller.kt (1)
3-9: LGTM!The interface is clean, focused, and provides a clear contract for S3 image upload URL generation.
pida-core/core-domain/src/main/kotlin/com/pida/blooming/BloomingImageUploadUrl.kt (1)
5-16: LGTM!The data class and factory method are well-structured, providing clear mapping from S3-specific types to domain types.
gradle/libs.versions.toml (1)
43-43: Verify Netty transitive dependencies for AWS SDK 2.40.13; confirm Bucket4j 8.15.0 stability.AWS SDK 2.40.13 (released Dec 20, 2025) may transitively depend on Netty versions vulnerable to CVE-2025-24970 (affects netty-handler 4.1.91–4.1.117; patched in 4.1.118+) and CVE-2023-44487 (affects Netty < 4.1.94). Verify the resolved io.netty version in your dependency tree (
./gradlew dependencies --configuration runtimeClasspath | grep netty) and ensure it is >= 4.1.118.Final; if not, add an explicit Netty override or upgrade to a later AWS SDK release that includes the patch.Bucket4j 8.15.0 is the latest stable version with no direct published CVEs. Run a software composition analysis (SCA) scan on your final artifact to check transitive dependencies.
pida-clients/aws-client/src/main/kotlin/com/pida/client/aws/s3/AwsS3Client.kt (1)
80-109: LGTM: Pagination logic correctly handles large object lists.The pagination implementation properly accumulates all objects across continuation tokens and builds a complete response.
pida-clients/aws-client/src/main/kotlin/com/pida/client/aws/image/ImageFileConstructor.kt
Show resolved
Hide resolved
pida-clients/aws-client/src/main/kotlin/com/pida/client/aws/image/ImageFileConstructor.kt
Show resolved
Hide resolved
pida-clients/aws-client/src/main/kotlin/com/pida/client/aws/image/ImageS3Processor.kt
Show resolved
Hide resolved
pida-clients/aws-client/src/main/kotlin/com/pida/client/aws/image/ImageS3Processor.kt
Show resolved
Hide resolved
pida-clients/aws-client/src/main/kotlin/com/pida/client/aws/s3/AwsS3Client.kt
Show resolved
Hide resolved
pida-core/core-api/src/main/kotlin/com/pida/presentation/v1/blooming/BloomingController.kt
Show resolved
Hide resolved
pida-core/core-domain/src/main/kotlin/com/pida/blooming/BloomingFacade.kt
Show resolved
Hide resolved
pida-core/core-domain/src/main/kotlin/com/pida/support/aws/PresignedUrlRateLimiter.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
pida-clients/aws-client/src/main/kotlin/com/pida/client/aws/s3/AwsS3Client.kt (2)
59-83: Critical path inconsistency and hard-coded values already flagged in previous review.This method has two issues that were identified in the previous review:
- Critical: Line 69 omits the profile prefix (
"$filePath/$fileName"), whilegenerateUrlandgetObjectAsBytesinclude it ("$profile/$filePath/$fileName"). This will break the upload/download flow.- Major: Lines 70-71 hard-code
"image/jpeg"andPUBLIC_READACL, limiting flexibility for multi-format support.Please address these issues as outlined in the previous review comments.
116-129: Using platform default charset for binary data is risky.Line 127 uses
Charset.defaultCharset()to convert S3 object bytes to a String. This is problematic because:
- The default charset is platform-dependent and can vary between environments
- For binary data (such as images), charset conversion doesn't make semantic sense
Consider refactoring based on the intended use case:
- If returning binary data: Change return type to
ByteArrayand remove charset conversion- If returning text data: Explicitly use
StandardCharsets.UTF_8instead of the platform default🔎 Proposed fixes
Option 1: For binary data (recommended for images)
fun getObjectAsBytes( bucketName: String, filePath: String, fileName: String, -): String { +): ByteArray { val request = GetObjectRequest .builder() .bucket(bucketName) .key("$profile/$filePath/$fileName") .build() - return s3Client.getObjectAsBytes(request).asString(Charset.defaultCharset()) + return s3Client.getObjectAsBytes(request).asByteArray() }Option 2: For text data with explicit charset
+import java.nio.charset.StandardCharsets + fun getObjectAsBytes( bucketName: String, filePath: String, fileName: String, ): String { val request = GetObjectRequest .builder() .bucket(bucketName) .key("$profile/$filePath/$fileName") .build() - return s3Client.getObjectAsBytes(request).asString(Charset.defaultCharset()) + return s3Client.getObjectAsBytes(request).asString(StandardCharsets.UTF_8) }
🧹 Nitpick comments (2)
pida-core/core-domain/src/main/kotlin/com/pida/support/aws/PresignedUrlRateLimiter.kt (1)
14-19: Good fix for the memory leak concern!The Caffeine cache with
expireAfterAccessandmaximumSizesuccessfully addresses the unbounded bucket map growth flagged in the previous review. The configuration parameters (10 minutes TTL, 100k max size) are reasonable defaults.💡 Optional: Externalize cache configuration
Consider externalizing the TTL and maximum size to application properties for operational flexibility:
@ConfigurationProperties(prefix = "pida.rate-limit.cache") data class RateLimitCacheProperties( val expireAfterAccessMinutes: Long = 10, val maximumSize: Long = 100_000 ) @Component class PresignedUrlRateLimiter( private val cacheProperties: RateLimitCacheProperties ) { private val buckets: Cache<Long, Bucket> = Caffeine .newBuilder() .expireAfterAccess(cacheProperties.expireAfterAccessMinutes, TimeUnit.MINUTES) .maximumSize(cacheProperties.maximumSize) .build() // ... }gradle/libs.versions.toml (1)
62-66: Versions are current and secure; consider SCA scanning for Bucket4j transitive dependencies.Bucket4j 8.15.0 (released Aug 24, 2025) and Caffeine 3.2.3 (released Oct 28, 2025) are both current stable releases with no direct CVEs. However, run an SCA scan on transitive dependencies for the specific Bucket4j modules used (e.g., backend connectors) to catch any inherited vulnerabilities.
Also applies to: 195-199
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
gradle/libs.versions.tomlpida-clients/aws-client/src/main/kotlin/com/pida/client/aws/s3/AwsS3Client.ktpida-core/core-domain/build.gradle.ktspida-core/core-domain/src/main/kotlin/com/pida/support/aws/PresignedUrlRateLimiter.kt
🧰 Additional context used
🧬 Code graph analysis (1)
pida-core/core-domain/build.gradle.kts (1)
build.gradle.kts (3)
implementation(43-56)libs(23-83)kotlin(58-65)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CI (21, 2.1.0, corretto)
🔇 Additional comments (6)
pida-clients/aws-client/src/main/kotlin/com/pida/client/aws/s3/AwsS3Client.kt (3)
17-24: LGTM: Environment variable reading is appropriate for profile-based prefixing.The companion object reads
SPRING_PROFILES_ACTIVEonce at class load time, which is appropriate since Spring profiles are typically set at application startup and don't change at runtime.
26-57: LGTM: Download URL generation is correctly implemented.The method properly includes the profile prefix in the S3 key and uses appropriate content-type for binary downloads.
85-114: Verify the intentional separation of profile prefix between read and write/list operations.The inconsistency exists but may be intentional. Analysis shows a pattern:
- Read operations (
generateUrl,getObjectAsBytes): include"$profile/$filePath/$fileName"- Write/List operations (
generateUploadUrl,getBucketListObjects): usefilePathdirectly without profileSince
generateUploadUrl(which is actively used) also omits the profile prefix, this suggests the design is intentional—profile prefixes organize existing files for reads, while write/list operations construct paths independently. Confirm whether this separation is the intended behavior.pida-core/core-domain/build.gradle.kts (1)
18-22: LGTM!The Bucket4j and Caffeine dependencies are correctly added with the appropriate
implementationscope and align with the version catalog declarations.pida-core/core-domain/src/main/kotlin/com/pida/support/aws/PresignedUrlRateLimiter.kt (1)
21-35: LGTM!The
consumeTokenmethod is well-implemented with lazy bucket initialization, proper token consumption logic, and appropriate error handling. The separation of the rate limit policy (PresignedUrlRateLimitPolicy.PER_MINUTE) follows good design principles.gradle/libs.versions.toml (1)
43-43: AWS SDK for Java version 2.40.13 is a valid release (published December 20, 2025) with no documented security vulnerabilities. The version upgrade is safe to proceed with.
char-yb
left a comment
char-yb
left a comment
There was a problem hiding this comment.
궁금한 점 위주로 코멘트 달아보았습니다!
덕분에 bucket4j와 Caffeine 캐시에 대해서 학습할 수 있었어요:)
추후 Caffeine과 bucket4j 활용도가 높아진다면 Configuration으로 전역 설정을 해볼 수도 있을 거 같네요
pida-clients/aws-client/src/main/kotlin/com/pida/client/aws/config/AwsConfig.kt
Show resolved
Hide resolved
pida-clients/aws-client/src/main/kotlin/com/pida/client/aws/config/AwsConfig.kt
Show resolved
Hide resolved
pida-core/core-domain/src/main/kotlin/com/pida/support/error/ErrorType.kt
Show resolved
Hide resolved
pida-core/core-domain/src/main/kotlin/com/pida/support/aws/PresignedUrlRateLimitPolicy.kt
Show resolved
Hide resolved
pida-core/core-domain/src/main/kotlin/com/pida/support/aws/PresignedUrlRateLimiter.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pida-core/core-domain/src/main/kotlin/com/pida/flowerspot/FlowerSpotFacade.kt (1)
36-49: Add comment documenting the intentional omission of image URLs in the list endpoint.The
findAllFlowerSpotmethod does not fetch imageUrls whilereadFlowerSpotDetailsdoes. This appears intentional for performance reasons (list views typically don't require S3 URL fetching for each spot), but it should be documented with a comment explaining this design choice to prevent future maintainers from inadvertently changing the behavior.
♻️ Duplicate comments (3)
pida-clients/aws-client/src/main/kotlin/com/pida/client/aws/image/ImageFileConstructor.kt (2)
23-29: Path injection risk remains unaddressed.The path construction still lacks validation for
prefixandprefixId, allowing potential path traversal attacks via characters like../or/.
27-27: Unsafe profile fallback could mix environments.The fallback to
"dev"remains unchanged and could result in production data being written to the dev prefix in S3.pida-clients/aws-client/src/main/kotlin/com/pida/client/aws/image/ImageS3Processor.kt (1)
28-34: 30-second TTL may be too restrictive for uploads.The presigned upload URL expires in 30 seconds, which may not provide sufficient time for users on slower connections or uploading larger images.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
pida-clients/aws-client/src/main/kotlin/com/pida/client/aws/image/ImageFileConstructor.ktpida-clients/aws-client/src/main/kotlin/com/pida/client/aws/image/ImageS3Processor.ktpida-clients/aws-client/src/main/kotlin/com/pida/client/aws/s3/AwsS3Client.ktpida-core/core-api/src/main/kotlin/com/pida/presentation/v1/flowerspot/response/FlowerSpotDetailsResponse.ktpida-core/core-domain/src/main/kotlin/com/pida/flowerspot/FlowerSpotDetails.ktpida-core/core-domain/src/main/kotlin/com/pida/flowerspot/FlowerSpotFacade.ktpida-core/core-domain/src/main/kotlin/com/pida/support/aws/ImageS3Caller.ktpida-core/core-domain/src/main/kotlin/com/pida/support/aws/PresignedUrlRateLimiter.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- pida-clients/aws-client/src/main/kotlin/com/pida/client/aws/s3/AwsS3Client.kt
🧰 Additional context used
🧬 Code graph analysis (1)
pida-core/core-domain/src/main/kotlin/com/pida/flowerspot/FlowerSpotDetails.kt (2)
pida-core/core-domain/src/main/kotlin/com/pida/flowerspot/FlowerSpot.kt (1)
id(5-16)pida-storage/db-core/src/main/kotlin/com/pida/storage/db/core/flowerspot/FlowerSpotEntity.kt (1)
name(16-47)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CI (21, 2.1.0, corretto)
🔇 Additional comments (7)
pida-core/core-domain/src/main/kotlin/com/pida/flowerspot/FlowerSpotDetails.kt (1)
19-40: LGTM! Clean addition of imageUrls support.The imageUrls property is properly added with sensible defaults and correctly propagated through the factory method.
pida-core/core-api/src/main/kotlin/com/pida/presentation/v1/flowerspot/response/FlowerSpotDetailsResponse.kt (1)
55-79: LGTM! Proper API response integration.The imageUrls field is correctly added with appropriate schema documentation and properly mapped from the domain model.
pida-core/core-domain/src/main/kotlin/com/pida/support/aws/ImageS3Caller.kt (1)
1-25: LGTM! Clean interface design.The interface clearly defines S3 image operations with appropriate method signatures. The mix of synchronous (
createUploadUrl) and suspending (getImageUrl) functions aligns with their respective use cases.pida-core/core-domain/src/main/kotlin/com/pida/flowerspot/FlowerSpotFacade.kt (1)
14-34: LGTM! Proper concurrent image URL fetching.The imageS3Caller integration correctly uses async to fetch image URLs concurrently with other data, and properly passes the results to the factory method.
pida-core/core-domain/src/main/kotlin/com/pida/support/aws/PresignedUrlRateLimiter.kt (1)
14-39: LGTM! Robust rate limiting implementation.The Caffeine-based cache with TTL and size limits properly addresses the memory leak concerns from previous reviews. The Bucket4j integration is correct, and the implementation appropriately throws an exception when rate limits are exceeded.
pida-clients/aws-client/src/main/kotlin/com/pida/client/aws/image/ImageS3Processor.kt (2)
42-53: LGTM! Clean conditional logic.The method correctly handles both single-file retrieval and directory listing cases with appropriate helper delegation.
67-82: LGTM! Proper S3 object listing.The method correctly filters out directory markers and extracts filenames before generating presigned URLs.
char-yb
left a comment
char-yb
left a comment
There was a problem hiding this comment.
레퍼런스와 답변 감사해용
imageUrls도 emptyList 처리된 것도 확인했습니다!
LGTM!
🌱 관련 이슈
📌 작업 내용 및 특이 사항
📝 참고
📌 체크 리스트
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.