Skip to content

Commit

Permalink
HHH-15669 Fix test failures when using Oracle 21
Browse files Browse the repository at this point in the history
Rings in Oracle polygons may be shifted depending on how it
is processed. The equality test now takes this into account.

Add test to investigate st_within test failure.
  • Loading branch information
maesenka committed Nov 22, 2022
1 parent bf128dd commit 6658c62
Show file tree
Hide file tree
Showing 13 changed files with 325 additions and 200 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/*
* Hibernate, Relational Persistence for Idiomatic Java
*
* License: GNU Lesser General Public License (LGPL), version 2.1 or later.
* See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
*/

package org.hibernate.spatial.dialect.oracle.hhh15669;


import java.util.List;

import org.hibernate.dialect.OracleDialect;
import org.hibernate.spatial.HibernateSpatialConfigurationSettings;
import org.hibernate.spatial.testing.SpatialSessionFactoryAware;

import org.hibernate.testing.TestForIssue;
import org.hibernate.testing.orm.junit.DomainModel;
import org.hibernate.testing.orm.junit.RequiresDialect;
import org.hibernate.testing.orm.junit.ServiceRegistry;
import org.hibernate.testing.orm.junit.SessionFactory;
import org.hibernate.testing.orm.junit.Setting;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import jakarta.persistence.Entity;
import jakarta.persistence.Id;
import jakarta.persistence.Table;
import org.geolatte.geom.G2D;
import org.geolatte.geom.Point;
import org.geolatte.geom.Polygon;

import static org.geolatte.geom.builder.DSL.g;
import static org.geolatte.geom.builder.DSL.point;
import static org.geolatte.geom.builder.DSL.polygon;
import static org.geolatte.geom.builder.DSL.ring;
import static org.geolatte.geom.crs.CoordinateReferenceSystems.WGS84;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.hibernate.testing.hamcrest.CollectionMatchers.hasSize;

@TestForIssue(jiraKey = "HHH-15669")
@DomainModel(annotatedClasses = { Foo.class })
@RequiresDialect(value = OracleDialect.class)
@SessionFactory
@ServiceRegistry(settings = {
@Setting(name = HibernateSpatialConfigurationSettings.ORACLE_OGC_STRICT, value = "true")
})
public class TestStWithinBug extends SpatialSessionFactoryAware {
Polygon<G2D> filter = polygon( WGS84, ring(
g( 0, 0 ),
g( 10, 0 ),
g( 10, 10 ),
g( 0, 10 ),
g( 0, 0 )
) );

@BeforeEach
public void before() {
scope.inTransaction(
session -> {
session.persist( new Foo( 1, point( WGS84, g( 5, 5 ) ) ) );
session.persist( new Foo( 2, point( WGS84, g( 12, 12 ) ) ) );
session.persist( new Foo( 3, point( WGS84, g( -1, -1 ) ) ) );
}
);
}


@Test
public void testHql() {
scope.inTransaction( session -> {

List<Foo> list = session
.createQuery( "from Foo where st_within(point, :filter) = true", Foo.class )
.setParameter( "filter", filter )
.getResultList();
assertThat( list, hasSize( 1 ) );
assertThat( list.get( 0 ).id, equalTo( 1L ) );
} );
}


@AfterEach
public void cleanup() {
scope.inTransaction( session -> session.createMutationQuery( "delete from Foo" )
.executeUpdate() );
}

}

@Entity(name = "Foo")
@Table(name = "Foo")
class Foo {
@Id
long id;
Point<G2D> point;

public Foo() {
}

public Foo(long id, Point<G2D> point) {
this.id = id;
this.point = point;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
import java.util.List;
import java.util.Map;

import org.geolatte.geom.GeometryEquality;

import org.hibernate.spatial.CommonSpatialFunction;
import org.hibernate.spatial.GeomCodec;
import org.hibernate.spatial.testing.TestSupportFactories;
Expand Down Expand Up @@ -38,6 +40,8 @@ public class SpatialTestDataProvider {
private final TestData funcTestData;
protected TestData testData;
protected GeomCodec codec;

protected GeometryEquality geometryEquality;
protected List<CommonSpatialFunction> exludeFromTest;

public SpatialTestDataProvider() {
Expand All @@ -51,6 +55,7 @@ public SpatialTestDataProvider() {
exludeFromTest = support.getExcludeFromTests();
funcTestData = support.createTestData( SpatialFunctionsData );
filterGeometry = support.getFilterGeometry();
geometryEquality = support.getGeometryEquality();
}
catch (InstantiationException | IllegalAccessException e) {
throw new RuntimeException( e );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public TestSupport.TestDataPurpose purpose() {
public Stream<DynamicTest> testFunction() {

return
TestTemplates.all( templates, hqlOverrides, filterGeometry )
TestTemplates.all( templates, hqlOverrides, geometryEquality, filterGeometry )
.filter( f -> isSupported( f.function ) )
.filter( f -> !exludeFromTest.contains( f.function ) )
.flatMap( t -> Stream.of(
Expand All @@ -91,7 +91,7 @@ protected Stream<DynamicTest> buildTests(FunctionTestTemplate template) {
) );
}

protected <T> String displayName(FunctionTestTemplate template, String fnName) {
protected String displayName(FunctionTestTemplate template, String fnName) {
return String.format(
Locale.ROOT,
"Test for function %s on entity %s",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.hibernate.testing.orm.junit.SessionFactoryScope;

import org.geolatte.geom.Geometry;
import org.geolatte.geom.GeometryEquality;
import org.geolatte.geom.codec.Wkt;

/**
Expand Down Expand Up @@ -115,7 +116,7 @@ public List executeHQL(SessionFactoryScope scope, String functionName) {
}
results.set( query.getResultList() );
} );
return (List) results.get().stream().map( rowObjectMapper::apply ).collect( Collectors.toList() );
return results.get().stream().map( rowObjectMapper::apply ).collect( Collectors.toList() );
}

//only for JtsGeometry because extra mapping of native Geometry object (where needed)
Expand Down Expand Up @@ -145,6 +146,8 @@ static class Builder {
RowObjectMapper mapper;
Geometry<?> testGeometry;

GeometryEquality geomEq;

public Builder(CommonSpatialFunction function) {
this.function = function;
}
Expand All @@ -163,7 +166,7 @@ else if ( function == CommonSpatialFunction.ST_BUFFER ) {
}
}
if ( this.mapper == null ) {
this.mapper = new RowObjectMapper() {
this.mapper = new RowObjectMapper( geomEq ) {
};
}
return new FunctionTestTemplate( function, hql, sql, mapper, model, testGeometry, codec );
Expand All @@ -187,5 +190,10 @@ Builder geometry(Geometry<?> value) {
}
return this;
}

public Builder equalityTest(GeometryEquality geomEq) {
this.geomEq = geomEq;
return this;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,25 +9,35 @@
import java.util.Arrays;
import java.util.Objects;

import org.geolatte.geom.GeometryEquality;
import org.geolatte.geom.jts.JTS;

/**
* Mapper to ensure that the results of the test queries can be compared for equality.
*
* @param <T> the returned object by the test query
*/
public interface RowObjectMapper<T> {
default Data apply(Object obj) {
public class RowObjectMapper {

final GeometryEquality geomEq;

RowObjectMapper(GeometryEquality geomEq) {
this.geomEq = geomEq;
}

Data apply(Object obj) {
Object[] row = (Object[]) obj;
return new Data( (Number) row[0], row[1] );
return new Data( (Number) row[0], row[1], geomEq );
}
}

class Data {
final Number id;
Object datum;
final GeometryEquality geomEq;

Data(Number id, Object datum) {
Data(Number id, Object datum, GeometryEquality geomEq) {
this.id = id;
this.datum = datum;
this.geomEq = geomEq;
}

@Override
Expand All @@ -42,18 +52,36 @@ public boolean equals(Object o) {
return Objects.equals( id.intValue(), data.id.intValue() ) && isEquals( datum, data.datum );
}

@SuppressWarnings("unchecked")
private boolean isEquals(Object thisDatum, Object thatDatum) {
if ( thisDatum instanceof byte[] ) {
if ( !( thatDatum instanceof byte[] ) ) {
return false;
}
return Arrays.equals( (byte[]) thisDatum, (byte[]) thatDatum );
}
if ( thisDatum instanceof org.geolatte.geom.Geometry ) {
return this.geomEq.equals( asGeolatte( thisDatum ), asGeolatte( thatDatum ) );
}

if ( thisDatum instanceof org.locationtech.jts.geom.Geometry ) {
return this.geomEq.equals( fromJts( thisDatum ), fromJts( thatDatum ) );
}
return Objects.equals( thisDatum, thatDatum );

}

@SuppressWarnings("rawtypes")
private org.geolatte.geom.Geometry asGeolatte(Object obj) {
return (org.geolatte.geom.Geometry) obj;
}

@SuppressWarnings("rawtypes")
private org.geolatte.geom.Geometry fromJts(Object obj) {
return JTS.from( (org.locationtech.jts.geom.Geometry) obj );
}


@Override
public int hashCode() {
return Objects.hash( id, datum );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,8 @@
import org.hibernate.spatial.CommonSpatialFunction;
import org.hibernate.spatial.testing.dialects.NativeSQLTemplates;

import org.geolatte.geom.G2D;
import org.geolatte.geom.Geometry;
import org.geolatte.geom.Polygon;

import static org.geolatte.geom.builder.DSL.g;
import static org.geolatte.geom.builder.DSL.polygon;
import static org.geolatte.geom.builder.DSL.ring;
import static org.geolatte.geom.crs.CoordinateReferenceSystems.WGS84;
import org.geolatte.geom.GeometryEquality;

/**
* Makes available all the builders for FunctionTestTemplate
Expand All @@ -42,7 +36,8 @@ static FunctionTestTemplate.Builder builder(CommonSpatialFunction function) {
public static Stream<FunctionTestTemplate.Builder> all(
NativeSQLTemplates sqlTemplates,
Map<CommonSpatialFunction, String> hqlOverrides,
Geometry<?>filter) {
GeometryEquality geomEq,
Geometry<?> filter) {

Map<CommonSpatialFunction, String> templates = sqlTemplates.all();
return templates
Expand All @@ -51,6 +46,7 @@ public static Stream<FunctionTestTemplate.Builder> all(
.map( function -> builder( function )
.hql( hqlOverrides.get( function ) )
.sql( templates.get( function ) )
.equalityTest( geomEq )
.geometry( setFilter( function ) ? filter : null ) );

}
Expand Down

This file was deleted.

0 comments on commit 6658c62

Please sign in to comment.