Skip to content
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.

Sign up for GitHub

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: silently ignores exception when parameter setting to count query #781

Merged
merged 9 commits into from
Oct 15, 2024
Merged

fix: silently ignores exception when parameter setting to count query #781

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
13f5e17
fix: silently ignores exception when parameter setting to count query
bagger3025 Oct 11, 2024
fe2b4b4
fix: use map instead of try-catch
bagger3025 Oct 11, 2024
e914ced
fix: test error fix and change map to set
bagger3025 Oct 11, 2024
20cf102
fix: revert divided methods into one method
bagger3025 Oct 11, 2024
f9af12c
fix: debug logging when parameter binding is skipped
bagger3025 Oct 11, 2024
5be754e
fix: minor grammar fix
bagger3025 Oct 11, 2024
78b44f6
fix: apply exception ignore to all in support
bagger3025 Oct 12, 2024
2e58f1e
refactor: logging with string template
bagger3025 Oct 14, 2024
fe8ff03
refactor: debug logic into else statement
bagger3025 Oct 15, 2024
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
6 changes: 3 additions & 3 deletions ...c/main/kotlin/com/linecorp/kotlinjdsl/support/eclipselink/javax/JpqlEntityManagerUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,9 @@ internal object JpqlEntityManagerUtils {
query.setParameter(name, value)
} else if (log.isDebugEnabled) {
Copy link
Member

Choose a reason for hiding this comment

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

이건 개인적인 취향이기 때문에 반영해주셔도 되고 안 해주셔도 됩니다.

개인적으로는 if else의 레벨을 맞추는 것을 선호합니다.
parameterNameSet에 name이 포함되어 있는지 여부와 log가 debug 레벨인지 여부는 같은 레벨의 비지니스로 보기 어렵다고 생각합니다.

그래서 개인적으로는 else 블록 안에서 if를 추가하는 것을 선호합니다.
그러면 코드를 해석할 때, 파라미터 이름이 없으면 다른 작업을 하게 되는데 그 중에서 로그 레벨이 debug면 로그를 출력한다와 같이 읽을 수 있기 때문입니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

이거는 저도 조금 이상해 보이긴 했습니다 저는 수정하는게 좋을것 같습니다

Copy link
Contributor Author

@bagger3025 bagger3025 Oct 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

else 안에 if 들어가는 코드여서 바깥으로 뺐는데, 말씀해주신 대로 코드를 바꾸는 게 맞는 것 같습니다.

수정해서 커밋하겠습니다!

log.debug(
"No parameter named '{}' in query with named parameters [{}], parameter binding skipped",
name,
parameterNameSet.joinToString(),
"No parameter named '$name' in query " +
cj848 marked this conversation as resolved.
Show resolved Hide resolved
"with named parameters [${parameterNameSet.joinToString()}], " +
"parameter binding skipped",
)
}
}
Expand Down
6 changes: 3 additions & 3 deletions ...ink/src/main/kotlin/com/linecorp/kotlinjdsl/support/eclipselink/JpqlEntityManagerUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,9 @@ internal object JpqlEntityManagerUtils {
query.setParameter(name, value)
} else if (log.isDebugEnabled) {
log.debug(
"No parameter named '{}' in query with named parameters [{}], parameter binding skipped",
name,
parameterNameSet.joinToString(),
"No parameter named '$name' in query " +
"with named parameters [${parameterNameSet.joinToString()}], " +
"parameter binding skipped",
)
}
}
Expand Down
6 changes: 3 additions & 3 deletions ...javax/src/main/kotlin/com/linecorp/kotlinjdsl/support/hibernate/JpqlEntityManagerUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ internal object JpqlEntityManagerUtils {
query.setParameter(name, value)
} else if (log.isDebugEnabled) {
log.debug(
"No parameter named '{}' in query with named parameters [{}], parameter binding skipped",
name,
parameterNameSet.joinToString(),
"No parameter named '$name' in query " +
"with named parameters [${parameterNameSet.joinToString()}], " +
"parameter binding skipped",
)
}
}
Expand Down
6 changes: 3 additions & 3 deletions ...rnate/src/main/kotlin/com/linecorp/kotlinjdsl/support/hibernate/JpqlEntityManagerUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ internal object JpqlEntityManagerUtils {
query.setParameter(name, value)
} else if (log.isDebugEnabled) {
log.debug(
"No parameter named '{}' in query with named parameters [{}], parameter binding skipped",
name,
parameterNameSet.joinToString(),
"No parameter named '$name' in query " +
"with named parameters [${parameterNameSet.joinToString()}], " +
"parameter binding skipped",
)
}
}
Expand Down
10 changes: 5 additions & 5 deletions .../main/kotlin/com/linecorp/kotlinjdsl/support/spring/batch/javax/JpqlEntityManagerUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,16 @@ internal object JpqlEntityManagerUtils {
}

private fun setParams(query: Query, params: JpqlRenderedParams) {
val parameterList = query.parameters.map { it.name }.toHashSet()
val parameterNameSet = query.parameters.map { it.name }.toHashSet()

params.forEach { (name, value) ->
if (parameterList.contains(name)) {
if (parameterNameSet.contains(name)) {
query.setParameter(name, value)
} else if (log.isDebugEnabled) {
log.debug(
"No parameter named '{}' in query with named parameters [{}], parameter binding skipped",
name,
parameterList.joinToString(),
"No parameter named '$name' in query " +
"with named parameters [${parameterNameSet.joinToString()}], " +
"parameter binding skipped",
)
}
}
Expand Down
6 changes: 3 additions & 3 deletions ...ch/src/main/kotlin/com/linecorp/kotlinjdsl/support/spring/batch/JpqlEntityManagerUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ internal object JpqlEntityManagerUtils {
query.setParameter(name, value)
} else if (log.isDebugEnabled) {
log.debug(
"No parameter named '{}' in query with named parameters [{}], parameter binding skipped",
name,
parameterNameSet.joinToString(),
"No parameter named '$name' in query " +
"with named parameters [${parameterNameSet.joinToString()}], " +
"parameter binding skipped",
)
}
}
Expand Down
10 changes: 5 additions & 5 deletions ...in/kotlin/com/linecorp/kotlinjdsl/support/spring/data/jpa/javax/JpqlEntityManagerUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,16 +98,16 @@ internal object JpqlEntityManagerUtils {
}

private fun setParams(query: Query, params: Map<String, Any?>) {
shouwn marked this conversation as resolved.
Show resolved Hide resolved
val parameterList = query.parameters.map { it.name }.toHashSet()
val parameterNameSet = query.parameters.map { it.name }.toHashSet()

params.forEach { (name, value) ->
if (parameterList.contains(name)) {
if (parameterNameSet.contains(name)) {
query.setParameter(name, value)
} else if (log.isDebugEnabled) {
log.debug(
"No parameter named '{}' in query with named parameters [{}], parameter binding skipped",
name,
parameterList.joinToString(),
"No parameter named '$name' in query " +
"with named parameters [${parameterNameSet.joinToString()}], " +
"parameter binding skipped",
)
}
}
Expand Down
10 changes: 5 additions & 5 deletions ...src/main/kotlin/com/linecorp/kotlinjdsl/support/spring/data/jpa/JpqlEntityManagerUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,16 +98,16 @@ internal object JpqlEntityManagerUtils {
}

private fun setParams(query: Query, params: Map<String, Any?>) {
val parameterList = query.parameters.map { it.name }.toHashSet()
val parameterNameSet = query.parameters.map { it.name }.toHashSet()

params.forEach { (name, value) ->
if (parameterList.contains(name)) {
if (parameterNameSet.contains(name)) {
query.setParameter(name, value)
} else if (log.isDebugEnabled) {
log.debug(
"No parameter named '{}' in query with named parameters [{}], parameter binding skipped",
name,
parameterList.joinToString(),
"No parameter named '$name' in query " +
"with named parameters [${parameterNameSet.joinToString()}], " +
"parameter binding skipped",
)
}
}
Expand Down
Loading