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

Consider JTS Geometry types as simple types #1711

Closed
pete-setchell-kubra opened this issue Jan 4, 2024 · 5 comments
Closed

Consider JTS Geometry types as simple types #1711

pete-setchell-kubra opened this issue Jan 4, 2024 · 5 comments
Labels
type: enhancement A general enhancement

Comments

@pete-setchell-kubra
Copy link
Contributor

pete-setchell-kubra commented Jan 4, 2024

Bug Report

Creating a basic entity with a PostGIS geometry Point type does not function as expected with Spring Data R2DBC. A Point can be read from a fixture in the database using the native PostgisGeometryCodec, but cannot be inserted in a new entity.

Versions

  • Driver: 1.0.3.RELEASE (also functioned the same rolling back through 0.8.5.RELEASE)
  • Database: postgres (PostgreSQL) 14.10 (Homebrew)
  • Java: OpenJDK 64-Bit Server VM (build 21.0.1+12-29, mixed mode, sharing)
  • OS: MacOS

Current Behavior

Attempting to set up a minimal mapping of a point with JTS does not work as expected with kotlin and Spring Data R2DBC. Running through a debugger, the PostgisGeometryCodec seems to be dynamically loaded during connection handshake, but this is not done in time to stop the exception being thrown on first write.

Stack trace
org.springframework.dao.InvalidDataAccessApiUsageException: Nested entities are not supported

	at org.springframework.data.r2dbc.convert.MappingR2dbcConverter.writePropertyInternal(MappingR2dbcConverter.java:251)
	at org.springframework.data.r2dbc.convert.MappingR2dbcConverter.writeProperties(MappingR2dbcConverter.java:218)
	at org.springframework.data.r2dbc.convert.MappingR2dbcConverter.writeInternal(MappingR2dbcConverter.java:189)
	at org.springframework.data.r2dbc.convert.MappingR2dbcConverter.write(MappingR2dbcConverter.java:181)
	at org.springframework.data.r2dbc.convert.MappingR2dbcConverter.write(MappingR2dbcConverter.java:61)
	at org.springframework.data.r2dbc.core.DefaultReactiveDataAccessStrategy.getOutboundRow(DefaultReactiveDataAccessStrategy.java:177)
	at org.springframework.data.r2dbc.core.R2dbcEntityTemplate.lambda$doInsert$6(R2dbcEntityTemplate.java:470)
	at reactor.core.publisher.FluxFlatMap.trySubscribeScalarMap(FluxFlatMap.java:153)
	at reactor.core.publisher.MonoFlatMap.subscribeOrReturn(MonoFlatMap.java:53)
	at reactor.core.publisher.InternalMonoOperator.subscribe(InternalMonoOperator.java:63)
	at reactor.core.publisher.MonoUsingWhen.subscribe(MonoUsingWhen.java:87)
	at reactor.core.publisher.Mono.subscribe(Mono.java:4512)
	at kotlinx.coroutines.reactor.MonoKt.awaitSingleOrNull(Mono.kt:47)
	at org.springframework.aop.framework.CoroutinesUtils.awaitSingleOrNull(CoroutinesUtils.java:42)
	at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:268)
	at jdk.proxy2/jdk.proxy2.$Proxy69.save(Unknown Source)
	at com.example.r2dbchibernatepoc.repository.AssetRepositoryTest$testInsert$1.invokeSuspend(AssetRepositoryTest.kt:29)
	at _COROUTINE._BOUNDARY._(CoroutineDebugging.kt:46)
	at com.example.r2dbchibernatepoc.repository.AssetRepositoryTest$testInsert$1.invokeSuspend(AssetRepositoryTest.kt:29)```
</details>
 
#### Table schema

<!--- Provide the table schema. -->

<details>
<summary>Input Code</summary>

```sql
CREATE SEQUENCE asset_id_seq;
CREATE TABLE IF NOT EXISTS asset (
    id BIGINT PRIMARY KEY DEFAULT NEXTVAL('asset_id_seq'),
    geom GEOMETRY(Point, 4326)
);

Steps to reproduce

Input Code
import org.locationtech.jts.geom.Geometry

@Table("asset")
data class Asset constructor(
    @Id val id: Long?,
    @Column val geom: Geometry
)

interface AssetRepository : CoroutineCrudRepository<Asset, Long> {
    override suspend fun findById(id: Long): Asset?
}
import org.locationtech.jts.geom.Coordinate;
import org.locationtech.jts.geom.Geometry
import org.locationtech.jts.geom.GeometryFactory;
import org.locationtech.jts.geom.PrecisionModel

@SpringBootTest
class AssetRepositoryTest @Autowired constructor(val userRepository: UserRepository, val assetRepository: AssetRepository) {

    @Test
    fun testInsert() {
        val longitude = 37.7434579544699
        val latitude = -122.44437070120628
        val factory4326 = GeometryFactory(PrecisionModel(PrecisionModel.FLOATING), 4326)
        val p: Geometry = factory4326.createPoint(Coordinate(longitude, latitude)) as Geometry

        runBlocking {
            val a = assetRepository.save(Asset(id=null, geom = p))
            assertNotNull(a.id)
            val aid = a.id!!
            val b = assetRepository.findById(aid)
            assertEquals(a, b)
            assertNotNull(b?.geom)
        }
    }

    @Test
    fun testRead() {
        runBlocking {
            val aid = 1L
            val a = assetRepository.findById(aid)
            assertNotNull(a?.geom)
        }
    }
}

A minimal reproducible version of this bug is available here: https://github.com/pete-setchell-kubra/r2dbcrepro

Expected behavior/code

  • testRead runs and loads an entity from a fixture, mapping the geometry correctly.
  • testInsert fails with the exception about Nested entity.

Possible Solution

This may be in issue in r2dbc-postgresql, I'll double file there and update this ticket with an issue number.

@pete-setchell-kubra
Copy link
Contributor Author

Also filed against r2dbc-postgresql

pgjdbc/r2dbc-postgresql#626

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 4, 2024
@mp911de
Copy link
Member

mp911de commented Jan 5, 2024

We do not consider JTS types (org.locationtech.jts) as simple ones as Spring Data has no dependency on JTS.

I suggest subclassing org.springframework.data.r2dbc.dialect.PostgresDialect and providing JTS types via getSimpleTypes() in addition to PostgresDialect.getSimpleTypes().

As we're not deeply involved with JTS, I suggest that you first experiment within your own domain and once you're happy with how things work, feel free to submit a pull request.

@mp911de mp911de added status: ideal-for-contribution An issue that a contributor can help us with type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 5, 2024
@mp911de mp911de changed the title Spring Data R2DBC and Native PostGIS Geometry type: Nested entities are not supported Consider JTS Geometry types as simple types Jan 5, 2024
@pete-setchell-kubra
Copy link
Contributor Author

Thanks for the pointer, I'm attempting to implement a PostgisDialect as you suggest.

I'm curious about the distinction between org.locationtech.jts types and io.r2dbc.postgresql.codec types that you do support. org.springframework.data.r2dbc.dialect.PostgresDialect is obviously designed to define the types that r2dbc-postgresql supports, and does so by instrospection. As org.locationtech.jts is at least a transitive provided dependency of r2dbc-postgresql couldn't you follow a similar approach without having to introduce dependencies?

@pete-setchell-kubra
Copy link
Contributor Author

For anyone else trying to get this working, here's my working kotlin solution.

class PostgisDialect: PostgresDialect() {

    companion object {
        var gisSimpleTypes: MutableCollection<out Class<*>>

        init {
            val types = HashSet(org.springframework.data.r2dbc.dialect.PostgresDialect.INSTANCE.simpleTypes())
            types.addAll(listOf<String> (
                    "org.locationtech.jts.geom.Geometry",
                    "org.locationtech.jts.geom.Point",
                    "org.locationtech.jts.geom.MultiPoint",
                    "org.locationtech.jts.geom.LineString",
                    "org.locationtech.jts.geom.LinearRing",
                    "org.locationtech.jts.geom.MultiLineString",
                    "org.locationtech.jts.geom.Polygon",
                    "org.locationtech.jts.geom.MultiPolygon",
                    "org.locationtech.jts.geom.GeometryCollection"
                )
                .filter {
                    ClassUtils.isPresent(it, PostgisDialect::class.java.getClassLoader())
                }.map {
                    ClassUtils.resolveClassName(it, PostgisDialect::class.java.getClassLoader())
                }.toMutableSet())
            gisSimpleTypes = types
        }
    }

    override fun getSimpleTypes(): MutableCollection<out Class<*>> {
        return gisSimpleTypes
    }
}
class PostgisDialectProvider: R2dbcDialectProvider {
    override fun getDialect(connectionFactory: ConnectionFactory): Optional<R2dbcDialect> {
        return Optional.of(PostgisDialect())
    }
}

META-INF/spring.factories

org.springframework.data.r2dbc.dialect.DialectResolver$R2dbcDialectProvider=com.example.r2dbchibernatepoc.repository.PostgisDialectProvider

@mp911de, I'll prepare a pull request to roll this up. As it doesn't require converters or any additional dependencies, I don't see why it couldn't be rolled straight into PostgresDialect.

@pete-setchell-kubra
Copy link
Contributor Author

This turned out to be extremely straightforward, so I've created a pull request. Hope it saves someone else an hour or two of debugging!

#1713

@mp911de mp911de linked a pull request Jan 8, 2024 that will close this issue
@mp911de mp911de closed this as completed in ebdbe04 Jan 8, 2024
mp911de added a commit that referenced this issue Jan 8, 2024
Add Geometry super-type only as subtypes are considered simple types already. Ensure reflective simple type AOT registration.

Original pull request: #1713
See #1711
mp911de pushed a commit that referenced this issue Jan 8, 2024
mp911de added a commit that referenced this issue Jan 8, 2024
Add Geometry super-type only as subtypes are considered simple types already. Ensure reflective simple type AOT registration.

Original pull request: #1713
See #1711
mp911de pushed a commit that referenced this issue Jan 8, 2024
mp911de added a commit that referenced this issue Jan 8, 2024
Add Geometry super-type only as subtypes are considered simple types already. Ensure reflective simple type AOT registration.

Original pull request: #1713
See #1711
@mp911de mp911de removed the status: ideal-for-contribution An issue that a contributor can help us with label Jan 8, 2024
@mp911de mp911de added this to the 3.1.8 (2023.0.8) milestone Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants