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.

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

@BatchMapping does not support kotlin collection whose element type is interface. #454

Open
babyfish-ct opened this issue Jul 28, 2022 · 6 comments
Labels
status: waiting-for-triage An issue we've not yet triaged

Comments

@babyfish-ct
Copy link

babyfish-ct commented Jul 28, 2022

The annotation @BatchMapping requires method whose parameter is collection/list, eg

@BatchMapping
public Map<Child, Parent> manyToOne(List<Child> childObjects) {...} 

For java, it's OK.

Howerver, for kotlin, it's not OK

@BatchMapping
fun manyToOne(childObjects: List<Child>): Map<Child, Parent> =...

Because kotlin compiler generates the parameter type as kotlin.collections.List/kotlin.collections.Collection, not java.util.List/java.util.Collection

Now, my workaround is: Explicitly use java.util.List in kotlin, this will cause warning of Intellij.


SpringBoot version:
2.7.0

Example code:
Line 36 and 46 of here

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 28, 2022
@rstoyanchev
Copy link
Contributor

At the end of the day, we have to adapt to BatchLoader and MappedBatchLoader from the underlying java-dataloader library and that expects java List and Map. At best we could allow use of Kotlin Collection and Map but we'd have to copy those into a java List and Map.

@babyfish-ct
Copy link
Author

In kotlin language, kotlin.collections.List and java.util.List are different types.

However, that's only a language-level magic, after compiling, in JVM byte code, kotlin.collections.List will be translated to java.util.List

interface Example {
    fun f(values: kotlin.collections.List<String>)
}

fun main(args: Array<String>) {
    val javaMethod = Example::class.java.getMethod("f", List::class.java)
    println(javaMethod)
}

Output: public abstract void com.example.demo.Example.f(java.util.List)

@david-kubecka
Copy link

@babyfish-ct What problem do you face when you use the kotlin.collections.List in @BatchMapping method? We do exactly that in our Kotlin app and there seem to be no issues.

@babyfish-ct
Copy link
Author

babyfish-ct commented Oct 29, 2022

OK, if the data types are classes, it works as you said, there is no issues

However, if data types are interfaces, it cannot work

Minimized code

package issue.demo

import org.springframework.boot.autoconfigure.SpringBootApplication
import org.springframework.boot.runApplication
import org.springframework.graphql.data.method.annotation.BatchMapping
import org.springframework.graphql.data.method.annotation.QueryMapping
import org.springframework.stereotype.Controller

@SpringBootApplication
class DemoApplication

fun main(args: Array<String>) {
	runApplication<DemoApplication>(*args)
}

// Interface, not class
interface Order {
	val id: Int
	val name: String
}

// Interface, not class
interface OrderItem {
	val id: Int
	val name: String
}

@Controller
class OrderController {

	@QueryMapping
	fun orders(): List<Order> = ORDERS.values.toList()

	@BatchMapping
	fun items(
		// List: Not OK
		// java.util.List: OK
		orders: List<Order>
	): Map<Order, List<OrderItem>> =
		orders.associateBy({it}) { order ->
			ORDER_ITEM_MAPPINGS
				.filter { it.first == order.id }
				.mapNotNull { ORDER_ITEMS[it.second] }
		}
}

data class OrderImpl(
	override val id: Int,
	override val name: String
) : Order

data class OrderItemImpl(
	override val id: Int,
	override val name: String
) : OrderItem

val ORDERS = listOf<Order>(
	OrderImpl(1, "order-1"),
	OrderImpl(2, "order-2")
).associateBy {
	it.id
}

val ORDER_ITEMS = listOf<OrderItem>(
	OrderItemImpl(1, "order-1-1"),
	OrderItemImpl(2, "order-1-2"),
	OrderItemImpl(4, "order-2-1"),
	OrderItemImpl(3, "order-2-2"),
).associateBy {
	it.id
}

val ORDER_ITEM_MAPPINGS = listOf(
	1 to 1,
	2 to 1,
	3 to 2,
	4 to 2
)

@rstoyanchev
Copy link
Contributor

What's the actual error and stacktrace?

@babyfish-ct
Copy link
Author

babyfish-ct commented Oct 31, 2022

Error message is show in graphiq, not in java console.

  1. requsest
query {
  orders {
    id
    name
    items {
      id
      name
    }
  }
}
  1. response
{
  "errors": [
    {
      "message": "The field at path '/orders[0]/items' was declared as a non null type, but the code involved in retrieving data has wrongly returned a null value.  The graphql specification requires that the parent field be set to null, or if that is non nullable that it bubble up null to its parent and so on. The non-nullable type is '[OrderItem!]' within parent type 'Order'",
      "path": [
        "orders",
        0,
        "items"
      ],
      "extensions": {
        "classification": "NullValueInNonNullableField"
      }
    },
    {
      "message": "The field at path '/orders[1]/items' was declared as a non null type, but the code involved in retrieving data has wrongly returned a null value.  The graphql specification requires that the parent field be set to null, or if that is non nullable that it bubble up null to its parent and so on. The non-nullable type is '[OrderItem!]' within parent type 'Order'",
      "path": [
        "orders",
        1,
        "items"
      ],
      "extensions": {
        "classification": "NullValueInNonNullableField"
      }
    }
  ],
  "data": null
}
  1. sdl
type Query {
    orders: [Order!]!
}

type Order {
    id: Int!
    name: String!
    items: [OrderItem!]!
}

type OrderItem {
    id: Int!
    name: String!
}
  1. kotlin code is pasted in previous message.

If Order and OrderItem are classes, it works; if they are interfaces, cannot work.

@babyfish-ct babyfish-ct changed the title @BatchMapping does not support kotlin collection @BatchMapping does not support kotlin collection whose element type is interface. Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

No branches or pull requests

4 participants