From cd47b6877cee258113eb00523701a57374becfd8 Mon Sep 17 00:00:00 2001
From: Tuukka Lehtonen <tuukka.lehtonen@semantum.fi>
Date: Sun, 26 Aug 2018 15:28:01 +0300
Subject: [PATCH] Backported b75a6bbc for release/1.32.0.2

Previous too-optimized implementation had multiple flaws:
* Index calculation did not have enough parenthesis in it which meant
  that the value table index was sometimes calculated incorrectly.
  This lead to string literal values being read incorrectly.
* Did not take into account string literals longer than 127 characters
* Did not take into account that string literals are modified-utf-8
  encoded and simply decoded them as US-ASCII bytes. Essentially all
  resources with name containing special characters were indexed
  incorrectly up until now.

All of these flaws have been in the code for ages.

gitlab #86

Change-Id: Ibd5ecbaa1cdd1b8be759905bd8c29b9f8e012207
---
 .../internal/DirectQuerySupportImpl.java      | 60 ++++++++++++-------
 1 file changed, 38 insertions(+), 22 deletions(-)

diff --git a/bundles/org.simantics.db.procore/src/fi/vtt/simantics/procore/internal/DirectQuerySupportImpl.java b/bundles/org.simantics.db.procore/src/fi/vtt/simantics/procore/internal/DirectQuerySupportImpl.java
index d455861a3..fe992c4e9 100644
--- a/bundles/org.simantics.db.procore/src/fi/vtt/simantics/procore/internal/DirectQuerySupportImpl.java
+++ b/bundles/org.simantics.db.procore/src/fi/vtt/simantics/procore/internal/DirectQuerySupportImpl.java
@@ -32,6 +32,13 @@ import org.simantics.db.procore.cluster.ValueTableSmall;
 import org.simantics.db.request.AsyncRead;
 import org.simantics.db.service.DirectQuerySupport;
 
+/**
+ * Note that the direct value retrieval in this implementation only supports
+ * String-type literals - nothing else!
+ * 
+ * This implementation is mainly intended for optimizing database indexing
+ * performance.
+ */
 public class DirectQuerySupportImpl implements DirectQuerySupport {
 
 	final private SessionImplSocket session;
@@ -554,6 +561,7 @@ public class DirectQuerySupportImpl implements DirectQuerySupport {
 		
 	}
 	
+	@SuppressWarnings("unchecked")
 	private <T> void getValue4(final ReadGraphImpl graph, final ClusterImpl containerCluster, final int subject, final ForPossibleRelatedValueProcedure<T> procedure) {
 		
 		Object result = null;
@@ -648,6 +656,7 @@ public class DirectQuerySupportImpl implements DirectQuerySupport {
 	
 	}
 	
+	@SuppressWarnings("unchecked")
 	private <C, T> void getValue4(final ReadGraphImpl graph, final ClusterImpl containerCluster, final int subject, final C context, final ForPossibleRelatedValueContextProcedure<C, T> procedure) {
 		
 		Object result = null;
@@ -886,7 +895,8 @@ public class DirectQuerySupportImpl implements DirectQuerySupport {
 		
 		try {
 			byte[] bytes = cluster.getValue(subject, session.clusterTranslator);
-			T value = (T)utf(bytes);
+			@SuppressWarnings("unchecked")
+			T value = (T)utf(bytes, 0);
 			procedure.execute(graph, context, value);
 		} catch (DatabaseException e) {
 			procedure.execute(graph, context, null);
@@ -900,7 +910,8 @@ public class DirectQuerySupportImpl implements DirectQuerySupport {
 		
 		try {
 			byte[] bytes = cluster.getValue(subject, session.clusterTranslator);
-			T value = (T)utf(bytes);
+			@SuppressWarnings("unchecked")
+			T value = (T)utf(bytes, 0);
 			procedure.execute(graph, value);
 		} catch (DatabaseException e) {
 			procedure.execute(graph, null);
@@ -917,7 +928,8 @@ public class DirectQuerySupportImpl implements DirectQuerySupport {
 			if(bytes == null) {
 				procedure.execute(graph, context, null);
 			} else {
-				T value = (T)utf(bytes);
+				@SuppressWarnings("unchecked")
+				T value = (T)utf(bytes, 0);
 				procedure.execute(graph, context, value);
 			}
 		} catch (DatabaseException e) {
@@ -929,38 +941,42 @@ public class DirectQuerySupportImpl implements DirectQuerySupport {
 	}
 	
 	private <T> void getDirectValue4(final ReadGraphImpl graph, final ClusterSmall cluster, final int subject, final ForPossibleRelatedValueProcedure<T> procedure) {
+		try {
+			// Note: this code avoids creating an intermediate byte[]
+			// to store the encoded string bytes and reads the UTF string
+			// from the value table byte[] directly into String instead.
 
-		ResourceTableSmall rt = cluster.resourceTable;
-		ValueTableSmall vt = cluster.valueTable;
-
-		byte[] bs = vt.table;
-		long[] ls = rt.table;
+			ResourceTableSmall rt = cluster.resourceTable;
+			ValueTableSmall vt = cluster.valueTable;
 
-		int index = ((subject&0xFFFF) << 1) - 1 + rt.offset;
+			byte[] bs = vt.table;
+			long[] ls = rt.table;
 
-		int valueIndex = (int)(ls[index] >>> 24) & 0x3FFFFF + vt.offset;
+			int index = ((subject&0xFFF) << 1) - 1 + rt.offset;
+			int valueIndex = ((int)(ls[index] >>> 24) & 0x3FFFFF) + vt.offset;
 
-		int size = (int)bs[valueIndex++]-1;
-		char[] chars = new char[size];
-		valueIndex++;
-		for(int i=0;i<size;i++) {
-			chars[i] = (char)bs[valueIndex++];
-		}
+			int size = bs[valueIndex++];
+			if (size < 0) // two byte size
+				size = (int)(((size & 0x7F) << 8) | (bs[valueIndex++] & 0xFF));
+			if (size <= 0)
+				throw new DatabaseException("No value for " + subject); 
 
-		T value = (T)new String(chars);
+			@SuppressWarnings("unchecked")
+			T t = (T) utf(bs, valueIndex);
 
-		procedure.execute(graph, value);
-//		graph.dec();
-		
+			procedure.execute(graph, t);
+		} catch (DatabaseException e) {
+			procedure.exception(graph, e);
+		}
 	}
 
-	final private String utf(byte[] bytes) throws AssumptionException {
+	final private String utf(byte[] bytes, int offset) throws AssumptionException {
 
 		if(bytes == null) return null;
 
 		// Read databoard int32 using Length encoding
 		// https://dev.simantics.org/index.php/Databoard_Specification#Length
-		int index = 0;
+		int index = offset;
 		int length = bytes[index++]&0xff; 
 		if(length >= 0x80) {
 			if(length >= 0xc0) {
-- 
2.45.1