From 9e0b5b2b635176501a9cc80808ad2a0ed620bfcd Mon Sep 17 00:00:00 2001
From: Florian Kleedorfer <florian.kleedorfer@austria.fm>
Date: Mon, 3 Jun 2024 13:17:03 +0200
Subject: [PATCH] GH-5009: Fix position of OPTIONAL in RelationMapBuilder

---
 .../dao/support/RelationMapBuilder.java       | 23 ++++---
 .../dao/support/RelationMapBuilderTests.java  | 63 +++++++++++++++++++
 2 files changed, 76 insertions(+), 10 deletions(-)
 create mode 100644 spring-components/rdf4j-spring/src/test/java/org/eclipse/rdf4j/spring/dao/support/RelationMapBuilderTests.java

diff --git a/spring-components/rdf4j-spring/src/main/java/org/eclipse/rdf4j/spring/dao/support/RelationMapBuilder.java b/spring-components/rdf4j-spring/src/main/java/org/eclipse/rdf4j/spring/dao/support/RelationMapBuilder.java
index d4b982fbda8..24f60d596fb 100644
--- a/spring-components/rdf4j-spring/src/main/java/org/eclipse/rdf4j/spring/dao/support/RelationMapBuilder.java
+++ b/spring-components/rdf4j-spring/src/main/java/org/eclipse/rdf4j/spring/dao/support/RelationMapBuilder.java
@@ -146,14 +146,17 @@ private IRI getRelationValueOrNothing(BindingSet b) {
 
 	private TupleQueryEvaluationBuilder makeTupleQueryBuilder() {
 		return rdf4JTemplate
-				.tupleQuery(
-						Queries.SELECT(getProjection())
-								.where(getWhereClause())
-								.distinct()
-								.getQueryString())
+				.tupleQuery(makeQueryString())
 				.withBindings(bindingsBuilder.build());
 	}
 
+	String makeQueryString() {
+		return Queries.SELECT(getProjection())
+				.where(getWhereClause())
+				.distinct()
+				.getQueryString();
+	}
+
 	private Projectable[] getProjection() {
 		if (this.isSubjectKeyed) {
 			return new Projectable[] {
@@ -168,14 +171,14 @@ private Projectable[] getProjection() {
 
 	private GraphPattern[] getWhereClause() {
 		TriplePattern tp = _relSubject.has(predicate, _relObject);
+		GraphPattern[] ret = new GraphPattern[constraints.length + 1];
 		if (this.isRelationOptional) {
-			GraphPattern[] ret = new GraphPattern[constraints.length + 1];
-			ret[0] = tp.optional();
-			System.arraycopy(constraints, 0, ret, 1, constraints.length);
-			return ret;
+			ret[constraints.length] = tp.optional();
 		} else {
-			return new GraphPattern[] { tp.and(constraints) };
+			ret[constraints.length] = tp;
 		}
+		System.arraycopy(constraints, 0, ret, 0, constraints.length);
+		return ret;
 	}
 
 	public RelationMapBuilder withBinding(Variable key, Value value) {
diff --git a/spring-components/rdf4j-spring/src/test/java/org/eclipse/rdf4j/spring/dao/support/RelationMapBuilderTests.java b/spring-components/rdf4j-spring/src/test/java/org/eclipse/rdf4j/spring/dao/support/RelationMapBuilderTests.java
new file mode 100644
index 00000000000..d1024c6ec5d
--- /dev/null
+++ b/spring-components/rdf4j-spring/src/test/java/org/eclipse/rdf4j/spring/dao/support/RelationMapBuilderTests.java
@@ -0,0 +1,63 @@
+/*******************************************************************************
+ * Copyright (c) 2024 Eclipse RDF4J contributors.
+ *
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Distribution License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/org/documents/edl-v10.php.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ *******************************************************************************/
+package org.eclipse.rdf4j.spring.dao.support;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import org.eclipse.rdf4j.model.vocabulary.SKOS;
+import org.eclipse.rdf4j.spring.RDF4JSpringTestBase;
+import org.eclipse.rdf4j.spring.dao.support.RelationMapBuilder;
+import org.eclipse.rdf4j.spring.domain.model.EX;
+import org.eclipse.rdf4j.spring.support.RDF4JTemplate;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+
+public class RelationMapBuilderTests extends RDF4JSpringTestBase {
+
+	@Autowired
+	RDF4JTemplate rdf4JTemplate;
+
+	@Test
+	public void testRelationOnly() {
+		RelationMapBuilder rmb = new RelationMapBuilder(this.rdf4JTemplate, SKOS.BROADER);
+		assertEquals(normalize(
+				"SELECT DISTINCT ( ?rel_subject AS ?rel_key ) ( ?rel_object AS ?rel_value )\n"
+						+ "WHERE { ?rel_subject <http://www.w3.org/2004/02/skos/core#broader> ?rel_object . }"),
+				normalize(rmb.makeQueryString()));
+	}
+
+	@Test
+	public void testRelationWithConstraints() {
+		RelationMapBuilder rmb = new RelationMapBuilder(this.rdf4JTemplate, EX.creatorOf)
+				.constraints(RelationMapBuilder._relSubject.isA(
+						EX.Artist));
+		assertEquals(normalize("SELECT DISTINCT ( ?rel_subject AS ?rel_key ) ( ?rel_object AS ?rel_value )\n"
+				+ "WHERE { ?rel_subject a <http://example.org/Artist> .\n"
+				+ "?rel_subject <http://example.org/creatorOf> ?rel_object . }"), normalize(rmb.makeQueryString()));
+	}
+
+	@Test
+	public void testOptionalRelationWithPattern() {
+		RelationMapBuilder rmb = new RelationMapBuilder(this.rdf4JTemplate, EX.creatorOf)
+				.constraints(RelationMapBuilder._relSubject.isA(
+						EX.Artist))
+				.relationIsOptional();
+		assertEquals(normalize(
+				"SELECT DISTINCT ( ?rel_subject AS ?rel_key ) ( ?rel_object AS ?rel_value )\n"
+						+ "WHERE { ?rel_subject a <http://example.org/Artist> .\n"
+						+ "OPTIONAL { ?rel_subject <http://example.org/creatorOf> ?rel_object . } }\n"),
+				normalize(rmb.makeQueryString()));
+	}
+
+	private String normalize(String s) {
+		return s.replaceAll("\n", " ").replaceAll("\\s+", " ").trim();
+	}
+}