Add path method for `KFunction1` #521

jbl428 · 2023-11-12T01:32:08Z

Motivation

follow up PR of Add support for java getter #518

Modifications

Add path method for KFunction1
Add docs for java entity.

Commit Convention Rule

Please commit your modification based by conventional commit:

Commit type	Description
feat	New Feature
fix	Fix bug
docs	Documentation only changed
ci	Change CI configuration
refactor	Not a bug fix or add feature, just refactoring code
test	Add Test case or fix wrong test case
style	Only change the code style(ex. white-space, formatting)
chore	It refers to minor tasks such as library version upgrade, typo correction, etc.

If you want to add some more commit type please describe it on the Pull Request

Result

user can provide KFunction1 to path method

Closes

Support for java entity with private setter #517

codecov-commenter · 2023-11-12T02:41:08Z

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (85e973d) 92.11% compared to head (74ab44d) 91.90%.
Report is 2 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #521      +/-   ##
===========================================
- Coverage    92.11%   91.90%   -0.21%     
===========================================
  Files          269      269              
  Lines         2942     2966      +24     
  Branches       177      177              
===========================================
+ Hits          2710     2726      +16     
- Misses         181      189       +8     
  Partials        51       51

Files	Coverage Δ
...in/kotlin/com/linecorp/kotlinjdsl/dsl/jpql/Jpql.kt	`98.32% <100.00%> (+0.02%)`	⬆️
.../linecorp/kotlinjdsl/querymodel/jpql/path/Paths.kt	`100.00% <100.00%> (ø)`
.../com/linecorp/kotlinjdsl/property/PropertyUtils.kt	`100.00% <100.00%> (ø)`
...javax/autoconfigure/KotlinJdslAutoConfiguration.kt	`80.00% <66.66%> (-20.00%)`	⬇️
...batch/autoconfigure/KotlinJdslAutoConfiguration.kt	`80.00% <66.66%> (-20.00%)`	⬇️
...javax/autoconfigure/KotlinJdslAutoConfiguration.kt	`84.61% <60.00%> (-15.39%)`	⬇️
...a/jpa/autoconfigure/KotlinJdslAutoConfiguration.kt	`84.61% <60.00%> (-15.39%)`	⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

shouwn · 2023-11-12T04:42:30Z

dsl/jpql/src/main/kotlin/com/linecorp/kotlinjdsl/dsl/jpql/Jpql.kt

+     * Creates a path expression with the entity and property.
+     */
+    @SinceJdsl("3.1.0")
+    fun <T : Any, V> Entityable<T>.path(getter: KFunction1<T, @Exact V>): Path<V & Any> {


Is it possible to put in in the T generic type? I want to include in in something like KProperty1<in T, @Eact V> because an user might want to create a path with the parent type.

It works in the test, but it's probably because the receiver's T is cast to a supertype. This causes the type to be broken when chaining occurs.

when i first put in keyword. IDE show me the following warning.
It might that KFunction1 has in variance unlike KProperty1.

Projection is redundant: the corresponding type parameter of KFunction1 has the same variance

Aha, I see. Thank you for the confirmation.

shouwn · 2023-11-12T04:47:59Z

dsl/jpql/src/testFixtures/kotlin/com/linecorp/kotlinjdsl/dsl/jpql/entity/employee/Employee.kt

@@ -7,4 +7,6 @@ abstract class Employee(
    val phone: String,
    val address: EmployeeAddress,
    val departments: MutableSet<EmployeeDepartment>,
-)
+) {
+    fun getLowerName(): String = name.lowercase()


Since this is a test entity, it doesn't matter what methods it has, but how about making it a little more meaningful? Could we write code like fun getNickname() = nickname ?: name?

There is compile error when we have method named getNickname.
so i rename getDisplayName

Platform declaration clash: The following declarations have the same JVM signature (getNickname()Ljava/lang/String;)

displayName sounds really good.

shouwn · 2023-11-12T04:51:25Z

...src/testFixtures/kotlin/com/linecorp/kotlinjdsl/dsl/jpql/entity/employee/FullTimeEmployee.kt

@@ -17,4 +17,6 @@ class FullTimeEmployee(
    phone = phone,
    address = address,
    departments = departments,
-)
+) {
+    fun getUpperName(): String = name.uppercase()


I think we could make a method like fun getMonthlySalary(): EmployeeSalary = EmployeeSalary(annualSalary.value.div(BigDecimal.valueOf(12))).

shouwn · 2023-11-12T04:53:21Z

query-model/jpql/src/test/kotlin/com/linecorp/kotlinjdsl/querymodel/jpql/path/PathsTest.kt

@@ -33,6 +33,22 @@ class PathsTest : WithAssertions {
        assertThat(actual).isEqualTo(expected)
    }

+    @Test
+    fun pathGetter() {


Now that path can take two parameters, we can rename the path method to path with a property and rename it to path with a getter.

shouwn · 2023-11-12T04:56:38Z

docs/en/jpql-with-kotlin-jdsl/paths.md

@@ -13,6 +13,75 @@ entity(Book::class, "b").path(Book::isbn).path(Isbn::value)
 entity(Book::class, "b")(Book::isbn)(Isbn::value)
 ```

+## Java entity
+
+`path()` and `invoke()` take `KProperty1` or `KFuction1` as an argument.


How about replacing take with can take?

shouwn · 2023-11-12T05:00:54Z

docs/en/jpql-with-kotlin-jdsl/paths.md

+path(Book::getTitle)
+```
+
+Kotlin JDSL follows the following rules to infer property name from getter name.


It would be nice to change the sentence to something like this: Kotlin JDSL infers the property name from the getter with the following rules:

shouwn · 2023-11-12T05:03:45Z

docs/en/jpql-with-kotlin-jdsl/paths.md

+path(Book::getAvailable)
+```
+
+If you want to use your own rule instead of the above rules, you need to implement `JpqlPropertyIntrospector` and provide it to `RenderContext`.


I would like to change you need to implement to you can implement.

It would be nice to leave a guide and link to the Custom DSL for more details, and to Spring Support if the user is using Spring.

Also it would be nice to add Spring AutoConfigure, since there is no Spring Support for JpqlIntrospector. This forces the user to modify the JpqlRenderContext by themselves, which I think will be inconvenient for the user.

open class KotlinJdslAutoConfiguration { @Bean @ConditionalOnMissingBean @SinceJdsl("3.0.0") open fun jpqlRenderContext( serializers: List<JpqlSerializer<*>>, introspectors: List<JpqlIntrospector>, ): JpqlRenderContext { val userDefinedSerializers = object : JpqlRenderModule { override fun setupModule(context: JpqlRenderModule.SetupContext) { context.addAllSerializer(serializers.reversed()) introspectors.reversed().forEach { context.prependIntrospector(it) } } } return JpqlRenderContext() .registerModules(userDefinedSerializers) } // ... }

shouwn · 2023-11-12T05:12:17Z

docs/ko/jpql-with-kotlin-jdsl/paths.md

@@ -13,6 +13,75 @@ entity(Book::class, "b").path(Book::isbn).path(Isbn::value)
 entity(Book::class, "b")(Book::isbn)(Isbn::value)
 ```

+## Java entity


한국어까지 같이 작성해주셔서 정말 감사합니다. ❤️

query-model/jpql/src/test/kotlin/com/linecorp/kotlinjdsl/querymodel/jpql/path/PathsTest.kt

CLAassistant · 2023-11-12T12:44:39Z

All committers have signed the CLA.

cj848

Thank you for your contribution!

shouwn

Thanks for the help! I'll rename some of the tests.

jbl428 added 3 commits November 12, 2023 08:10

feat(query-model): add path methods for getter

341ddfc

feat(dsl): add path method for KFunction1

e5e3a00

docs: path for java entity

a8cd463

jbl428 requested review from pickmoment, huisam, cj848 and shouwn as code owners November 12, 2023 01:32

jbl428 changed the title ~~Feat/kfunction dsl~~ Add path method for KFunction1 Nov 12, 2023

shouwn reviewed Nov 12, 2023

View reviewed changes

jbl428 added 5 commits November 12, 2023 18:10

test: rename methods more meaningful

d0d35c4

docs: fix grammar

04cef12

feat: add introspectors to spring auto configuration

581d5d3

docs: add guides for spring

84fc35c

style: revert code style

6362262

cj848 reviewed Nov 12, 2023

View reviewed changes

query-model/jpql/src/test/kotlin/com/linecorp/kotlinjdsl/querymodel/jpql/path/PathsTest.kt Outdated Show resolved Hide resolved

test(query-model): rename test method

74ab44d

jbl428 force-pushed the feat/kfunction-dsl branch from 89e79c6 to 74ab44d Compare November 12, 2023 12:45

cj848 approved these changes Nov 12, 2023

View reviewed changes

shouwn approved these changes Nov 12, 2023

View reviewed changes

shouwn merged commit 6970af2 into line:develop Nov 12, 2023
3 checks passed

jbl428 deleted the feat/kfunction-dsl branch November 13, 2023 00:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add path method for `KFunction1` #521

Add path method for `KFunction1` #521

jbl428 commented Nov 12, 2023

codecov-commenter commented Nov 12, 2023 •

edited

Loading

shouwn Nov 12, 2023

shouwn Nov 12, 2023

jbl428 Nov 12, 2023

shouwn Nov 12, 2023

shouwn Nov 12, 2023

jbl428 Nov 12, 2023

shouwn Nov 12, 2023

shouwn Nov 12, 2023

shouwn Nov 12, 2023

shouwn Nov 12, 2023

shouwn Nov 12, 2023

shouwn Nov 12, 2023

shouwn Nov 12, 2023

shouwn Nov 12, 2023

shouwn Nov 12, 2023

CLAassistant commented Nov 12, 2023 •

edited

Loading

cj848 left a comment

shouwn left a comment •

edited

Loading

Add path method for KFunction1 #521

Add path method for KFunction1 #521

Conversation

jbl428 commented Nov 12, 2023

Motivation

Modifications

Commit Convention Rule

Result

Closes

codecov-commenter commented Nov 12, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CLAassistant commented Nov 12, 2023 • edited Loading

cj848 left a comment

Choose a reason for hiding this comment

shouwn left a comment • edited Loading

Choose a reason for hiding this comment

Add path method for `KFunction1` #521

Add path method for `KFunction1` #521

codecov-commenter commented Nov 12, 2023 •

edited

Loading

CLAassistant commented Nov 12, 2023 •

edited

Loading

shouwn left a comment •

edited

Loading