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 @@ -2,7 +2,11 @@ package com.pida.client.aws.s3

import org.springframework.stereotype.Component
import software.amazon.awssdk.services.s3.S3Client
import software.amazon.awssdk.services.s3.model.*
import software.amazon.awssdk.services.s3.model.GetObjectRequest
import software.amazon.awssdk.services.s3.model.ListObjectsV2Request
import software.amazon.awssdk.services.s3.model.ListObjectsV2Response
import software.amazon.awssdk.services.s3.model.PutObjectRequest
import software.amazon.awssdk.services.s3.model.S3Object
import software.amazon.awssdk.services.s3.presigner.S3Presigner
import software.amazon.awssdk.services.s3.presigner.model.GetObjectPresignRequest
import software.amazon.awssdk.services.s3.presigner.model.PutObjectPresignRequest
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class FcmClientRepository(
it.fcmToken,
it.title,
it.body,
it.destination,
)
}.toList()

Expand All @@ -24,6 +25,7 @@ class FcmClientRepository(
firebaseCloudMessageSender.sendEachForMulticastAll(
fcmRequests[0].title,
fcmRequests[0].body,
fcmRequests[0].destination,
tokens,
)

Expand All @@ -34,6 +36,7 @@ class FcmClientRepository(
fcmToken = pushMessage.fcmToken,
title = pushMessage.title,
body = pushMessage.body,
destination = pushMessage.destination,
tryCount = pushMessage.tryCount + 1,
sent = batchResult?.responses?.get(index)?.isSuccessful ?: false,
)
Expand All @@ -47,13 +50,15 @@ class FcmClientRepository(
fcmToken = firebaseCloudMessage.fcmToken,
title = firebaseCloudMessage.title,
body = firebaseCloudMessage.body,
destination = firebaseCloudMessage.destination,
)
val sendResult = firebaseCloudMessageSender.sendAsync(request).get()
return FirebaseCloudMessage(
fcmKey = firebaseCloudMessage.fcmKey,
fcmToken = firebaseCloudMessage.fcmToken,
title = firebaseCloudMessage.title,
body = firebaseCloudMessage.body,
destination = firebaseCloudMessage.destination,
tryCount = firebaseCloudMessage.tryCount + 1,
sent = sendResult.isNotBlank(),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@ data class FcmSendRequest(
val fcmToken: String,
val title: String,
val body: String,
val destination: String,
Copy link
Collaborator

Choose a reason for hiding this comment

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

์ด๊ฑด ์ œ๊ฐ€ FCM์— ๋Œ€ํ•œ ์ดํ•ด๊ฐ€ ๋ถ€์กฑํ•ด์„œ ์ผ ์ˆ˜๋„ ์žˆ๋Š”๋ฐ, destination๊ณผ topic์€ ์–ด๋–ค ์ฐจ์ด๊ฐ€ ์žˆ๋‚˜์š”?
๋‘˜๋‹ค ์ „์†ก ๊ฒฝ๋กœ๋ผ๋Š” ๋น„์Šทํ•œ ์˜๋ฏธ๋กœ ์•Œ๊ณ  ์žˆ์–ด์„œ ์—ฌ์ญค๋ด…๋‹ˆ๋‹ค!

Copy link
Member Author

Choose a reason for hiding this comment

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

destination์€ FCM ๊ด€๋ จํ•œ ํ•„๋“œ๊ฐ€ ์•„๋‹™๋‹ˆ๋‹ค!
putCustomData์œผ๋กœ APNS์—์„œ ์ €ํฌ๊ฐ€ ์ž„์˜๋กœ ์ปค์Šคํ…€ ํ•„๋“œ๋ฅผ ์ง€์ •ํ•  ์ˆ˜ ์žˆ๊ณ , ํ‘ธ์‹œ ์•Œ๋ฆผ์—์„œ UI ์ „ํ™˜์„ ์œ„ํ•œ ์•ฑ์Šคํ‚ค๋งˆ ์ปจํ…์ŠคํŠธ๋ผ๊ณ  ๋ด์ฃผ์‹ฌ ์ข‹์„ ๊ฑฐ ๊ฐ™์Šต๋‹ˆ๋‹ค!

Copy link
Collaborator

Choose a reason for hiding this comment

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

์•„ํ•˜ ๊ทธ๋ ‡๊ตฐ์š” ๋‹ต๋ณ€ ๊ฐ์‚ฌํ•ฉ๋‹ˆ๋‹ค!

)
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,11 @@ class FirebaseCloudMessageSender(
).setApnsConfig(
ApnsConfig
.builder()
.putCustomData("destination", request.destination)
.setAps(
Aps
.builder()
.setAlert("${request.title}\n${request.body}")
.setAlert(request.body)
.setBadge(1)
.setSound("default")
.build(),
Expand All @@ -48,6 +49,7 @@ class FirebaseCloudMessageSender(
fun sendEachForMulticastAll(
title: String,
body: String,
destination: String,
fcmTokens: List<String>,
): BatchResponse? {
val notification =
Expand All @@ -64,10 +66,11 @@ class FirebaseCloudMessageSender(
.setApnsConfig(
ApnsConfig
.builder()
.putCustomData("destination", destination)
.setAps(
Aps
.builder()
.setAlert("$title\n$body")
.setAlert(body)
.setBadge(1)
.setSound("default")
.build(),
Expand Down
2 changes: 0 additions & 2 deletions pida-core/core-api/src/main/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ spring.application.name: pida-server
spring.profiles.active: ${SPRING_PROFILES_ACTIVE}

spring:
main:
lazy-initialization: true
threads:
virtual:
enabled: true
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.pida.notification

data class CreateNotificationStoredCommand(
val notificationStoredKey: String,
val userId: Long,
val type: NotificationType,
val parameterValue: String,
val topic: String,
val contents: String,
val readStatus: ReadStatus = ReadStatus.UNREAD,
)
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class FcmSender(
fcmToken = it.fcmToken,
title = it.title,
body = it.body,
destination = it.destination,
tryCount = 0,
sent = false,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ data class FirebaseCloudMessage(
val fcmToken: String,
val title: String,
val body: String,
val destination: String,
Copy link

Choose a reason for hiding this comment

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

โš ๏ธ Potential issue | ๐Ÿ”ด Critical

๐Ÿงฉ Analysis chain

๐Ÿ Script executed:

#!/bin/bash
# First, find the FirebaseCloudMessage file and examine its structure
echo "=== FirebaseCloudMessage.kt file structure ==="
fd -t f "FirebaseCloudMessage.kt" --exec wc -l {} \;
echo ""

# Read the file to understand the class definition
echo "=== FirebaseCloudMessage.kt contents ==="
fd -t f "FirebaseCloudMessage.kt" --exec cat -n {} \;
echo ""

# Search for destination field usage in message construction and validation
echo "=== Destination field usage patterns ==="
rg -nP --type=kt -C3 'destination\s*[=:]' -g '!*Test.kt' || echo "No matches found"
echo ""

# Search for empty/blank checks on destination
echo "=== Empty/blank checks on destination ==="
rg -nP --type=kt 'destination\.(isNotBlank|isNotEmpty|isEmpty|isBlank)' -g '!*Test.kt' || echo "No matches found"
echo ""

# Search for where FirebaseCloudMessage is instantiated
echo "=== FirebaseCloudMessage instantiation ==="
rg -nP --type=kt 'FirebaseCloudMessage\s*\(' -C2 -g '!*Test.kt' | head -50

Repository: Team-PIDA/Pida-Server

Length of output: 1132


๐Ÿ Script executed:

#!/bin/bash
# Search for destination field usage without file type restriction
echo "=== Destination field usage in codebase ==="
rg -n 'destination' --type-list | grep -i kotlin || echo "Checking Kotlin type support..."
echo ""

# Try broader search
echo "=== All destination references ==="
rg -n '\.destination' -g '*.kt' -C2 | head -100
echo ""

# Search for FirebaseCloudMessage usage
echo "=== FirebaseCloudMessage instantiation/usage ==="
rg -n 'FirebaseCloudMessage(' -g '*.kt' -C3 | head -100
echo ""

# Search for validation in notification package
echo "=== Validation in notification package ==="
rg -n 'require\|check\|assert' pida-core/core-domain/src/main/kotlin/com/pida/notification/ | head -50
echo ""

# Check if there are any constructor validations
echo "=== Constructor or init blocks ==="
rg -n 'init\s*\{' pida-core/core-domain/src/main/kotlin/com/pida/notification/ -A5

Repository: Team-PIDA/Pida-Server

Length of output: 5667


๐Ÿ Script executed:

#!/bin/bash
# Examine the FirebaseCloudMessageSender implementation
echo "=== FirebaseCloudMessageSender.kt contents ==="
cat -n pida-clients/notification/src/main/kotlin/com/pida/client/notification/FirebaseCloudMessageSender.kt
echo ""

# Look for any custom data validation in related files
echo "=== NewFirebaseCloudMessage request creation ==="
rg -n 'NewFirebaseCloudMessage' pida-clients/notification/src/main/kotlin/com/pida/client/notification/ -A3 -B1
echo ""

# Check if there are any tests that show validation expectations
echo "=== Test files for destination ==="
find . -name '*Test.kt' -o -name '*Tests.kt' | xargs grep -l 'destination' 2>/dev/null | head -10
echo ""

# Examine any validation in request/domain models
echo "=== FcmRequest or related request models ==="
rg -n 'data class.*Request' pida-clients/notification/src/main/kotlin/com/pida/client/notification/ -A5

Repository: Team-PIDA/Pida-Server

Length of output: 4347


Add validation to ensure destination is not empty.

The destination field is sent directly as custom data in Firebase messages but has no validation. An empty string would be silently accepted and sent, causing routing failures on the client side. Add validation such as require(destination.isNotBlank()) in the constructor.

๐Ÿค– Prompt for AI Agents
In
pida-core/core-domain/src/main/kotlin/com/pida/notification/FirebaseCloudMessage.kt
around line 8, the destination property lacks validation and may be empty; add a
constructor check such as require(destination.isNotBlank()) (or
require(destination.isNotEmpty()) depending on style) to enforce
non-empty/non-blank values and throw an IllegalArgumentException if invalid,
ensuring the validation runs during object creation and prevents empty
destination values from being sent to Firebase.

val tryCount: Int,
val sent: Boolean,
)
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@ data class NewFirebaseCloudMessage(
val fcmToken: String,
val title: String,
val body: String,
val destination: String,
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package com.pida.notification

import com.pida.support.cursor.Cursor
import com.pida.support.cursor.CursorRequest
import org.springframework.stereotype.Service

@Service
class NotificationService(
private val notificationStoredAppender: NotificationStoredAppender,
private val notificationStoredReader: NotificationStoredReader,
private val notificationStoredUpdater: NotificationStoredUpdater,
private val notificationStoredKeyGenerator: NotificationStoredKeyGenerator,
) {
fun append(notificationStored: NotificationStored.Create): NotificationStored.Info =
notificationStoredAppender.append(
notificationStored.copy(
notificationStoredKey = notificationStoredKeyGenerator.generate(),
),
)

fun appendAll(createAll: List<CreateNotificationStoredCommand>): List<NotificationStored.Info> =
notificationStoredAppender.appendAll(
createAll.map { notification ->
NotificationStored.Create(
notificationStoredKey = notificationStoredKeyGenerator.generate(),
userId = notification.userId,
type = notification.type,
parameterValue = notification.parameterValue,
topic = notification.topic,
contents = notification.contents,
readStatus = ReadStatus.UNREAD,
)
},
)

fun findAllNotifications(
userId: Long,
cursorRequest: CursorRequest,
): Cursor<NotificationStored.Info> = notificationStoredReader.findAllBy(userId, cursorRequest)

fun markAsRead(notificationId: Long) = notificationStoredUpdater.markAsRead(notificationId)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package com.pida.notification

sealed class NotificationStored {
data class Create(
val notificationStoredKey: String,
val userId: Long,
val type: NotificationType,
val parameterValue: String,
val topic: String,
val contents: String,
val readStatus: ReadStatus,
)

data class Info(
val id: Long,
val notificationStoredKey: String,
val userId: Long,
val type: NotificationType,
val parameterValue: String,
val topic: String,
val contents: String,
val readStatus: ReadStatus,
val createdAt: String,
)

data class Result(
val id: Long,
val userId: Long,
val nickname: String?,
val spotName: String?,
val site: String?,
val type: String,
val parameterValue: String,
val topic: String,
val contents: String,
val readStatus: ReadStatus,
val createdAt: String,
)
Comment on lines +26 to +38
Copy link
Collaborator

Choose a reason for hiding this comment

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

์š”๊ธฐ Result๋Š” ์–ด๋–ค ์—ญํ• ์ธ๊ฐ€์š”~

Copy link
Member Author

Choose a reason for hiding this comment

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

์—ฌ๊ธฐ Result๋Š” Facade ๋ ˆ์ด์–ด์—์„œ User ๋„๋ฉ”์ธ๊ณผ Notification ๋„๋ฉ”์ธ์—์„œ ๋น„์ฆˆ๋‹ˆ์Šค ๋กœ์ง์„ ๊ฒฐํ•ฉํ•œ ๋ฐ์ดํ„ฐ ๋ชจ๋ธ์˜ ๊ฒฐ๊ณผ๋ผ๊ณ  ๋ด์ฃผ์‹ฌ ๋  ๊ฑฐ ๊ฐ™์Šต๋‹ˆ๋‹ค!

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package com.pida.notification

import org.springframework.stereotype.Component

@Component
class NotificationStoredAppender(
private val notificationStoredRepository: NotificationStoredRepository,
) {
fun append(create: NotificationStored.Create): NotificationStored.Info = notificationStoredRepository.save(create)

fun appendAll(createAll: List<NotificationStored.Create>) = notificationStoredRepository.saveAll(createAll)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package com.pida.notification

import org.springframework.stereotype.Component
import java.time.LocalDate
import java.time.format.DateTimeFormatter
import java.util.UUID

@Component
class NotificationStoredKeyGenerator {
companion object {
private val FORMAT_YYYYMMDD = DateTimeFormatter.ofPattern("yyyyMMdd")
}

fun generate(): String = "${generateDate()}_NHK_${generateUUID()}"

private fun generateUUID(): String = UUID.randomUUID().toString().replace("-", "")

private fun generateDate(): String = FORMAT_YYYYMMDD.format(LocalDate.now())
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package com.pida.notification

import com.pida.support.cursor.Cursor
import com.pida.support.cursor.CursorRequest
import org.springframework.stereotype.Component

@Component
class NotificationStoredReader(
private val notificationStoredRepository: NotificationStoredRepository,
) {
fun findAllBy(
userId: Long,
cursorRequest: CursorRequest,
): Cursor<NotificationStored.Info> = notificationStoredRepository.findAllBy(userId, cursorRequest)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package com.pida.notification

import com.pida.support.cursor.Cursor
import com.pida.support.cursor.CursorRequest

interface NotificationStoredRepository {
fun save(notificationStored: NotificationStored.Create): NotificationStored.Info

fun saveAll(createAll: List<NotificationStored.Create>): List<NotificationStored.Info>

fun findById(notificationStoredId: Long): NotificationStored.Info

fun findByUserIdAndReadStatus(
userId: Long,
readStatus: ReadStatus,
): List<NotificationStored.Info>

fun findAllBy(
userId: Long,
cursorRequest: CursorRequest,
): Cursor<NotificationStored.Info>

fun countByUserIdAndReadStatus(
userId: Long,
readStatus: ReadStatus?,
): Long

fun markAsRead(notificationId: Long)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package com.pida.notification

import org.springframework.stereotype.Component

@Component
class NotificationStoredUpdater(
private val notificationStoredRepository: NotificationStoredRepository,
) {
fun markAsRead(notificationId: Long) = notificationStoredRepository.markAsRead(notificationId)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.pida.notification

enum class NotificationType {
ADMIN_PUSH, // ๊ด€๋ฆฌ์ž ํ‘ธ์‹œ
REGULAR, // ์ •๊ธฐ ํ‘ธ์‹œ ์•Œ๋ฆผ (ํ™”์š”์ผ ์˜ค์ „ 9์‹œ)
;

companion object {
fun of(type: String): NotificationType =
when (type) {
"ADMIN_PUSH" -> ADMIN_PUSH
"REGULAR" -> REGULAR
else -> throw IllegalArgumentException("Unknown NotificationType: $type")
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.pida.notification

enum class ReadStatus {
READ,
UNREAD,
;

companion object {
fun of(readStatus: String): ReadStatus =
when (readStatus) {
"READ" -> READ
"UNREAD" -> UNREAD
else -> throw IllegalArgumentException("Unknown ReadStatus: $readStatus")
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package com.pida.support.cursor

data class Cursor<T>(
val content: List<T>,
val nextCursor: Long?,
val size: Long,
) {
companion object {
const val DEFAULT_CURSOR = 0L
const val DEFAULT_SIZE = 20

fun <T> of(
content: List<T>,
nextCursor: Long?,
size: Long,
): Cursor<T> {
require(size >= 0) { "size ($size)๋Š” 0 ์ด์ƒ์ด์–ด์•ผ ํ•ฉ๋‹ˆ๋‹ค" }
require(size >= content.size) {
"size ($size)๋Š” content.size (${content.size})๋ณด๋‹ค ์ž‘์„ ์ˆ˜ ์—†์Šต๋‹ˆ๋‹ค"
}
return Cursor(content, nextCursor, size)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package com.pida.support.cursor

data class CursorRequest(
val lastId: Long?,
val size: Long,
) {
init {
require(size in 1..100) { "ํฌ๊ธฐ๋Š” 1 ~ 100 ์‚ฌ์ด์—ฌ์•ผ ํ•ฉ๋‹ˆ๋‹ค." }
}
}
Loading
Loading