Mercurial > hg > monetdb-java

changeset 671:fade6c6960cc

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Refactor Handshake options It was not clear and there were obscure bugs.
author Joeri van Ruth <joeri.van.ruth@monetdbsolutions.com>
date Fri, 28 Oct 2022 16:16:00 +0200 (2022-10-28)
parents 3034312c1eda
children cd6193bc5956
files src/main/java/org/monetdb/jdbc/MonetConnection.java src/main/java/org/monetdb/mcl/net/HandshakeOption.java src/main/java/org/monetdb/mcl/net/HandshakeOptions.java src/main/java/org/monetdb/mcl/net/MapiSocket.java
diffstat 4 files changed, 169 insertions(+), 164 deletions(-) [+]
line wrap: on
line diff
--- a/src/main/java/org/monetdb/jdbc/MonetConnection.java
+++ b/src/main/java/org/monetdb/jdbc/MonetConnection.java
@@ -36,8 +36,7 @@ import java.util.concurrent.Executor;
 import org.monetdb.mcl.io.BufferedMCLReader;
 import org.monetdb.mcl.io.BufferedMCLWriter;
 import org.monetdb.mcl.io.LineType;
-import org.monetdb.mcl.net.HandshakeOptions;
-import org.monetdb.mcl.net.HandshakeOptions.Setting;
+import org.monetdb.mcl.net.HandshakeOption;
 import org.monetdb.mcl.net.MapiSocket;
 import org.monetdb.mcl.parser.HeaderLineParser;
 import org.monetdb.mcl.parser.MCLParseException;
@@ -166,7 +165,12 @@ public class MonetConnection
 	MonetConnection(final Properties props)
 		throws SQLException, IllegalArgumentException
 	{
-	// for debug: System.out.println("New connection object. Received properties are: " + props.toString());
+		HandshakeOption.AutoCommit autoCommitSetting = new HandshakeOption.AutoCommit(true);
+		HandshakeOption.ReplySize replySizeSetting = new HandshakeOption.ReplySize(DEF_FETCHSIZE);
+		HandshakeOption.SizeHeader sizeHeaderSetting = new HandshakeOption.SizeHeader(true);
+		HandshakeOption.TimeZone timeZoneSetting = new HandshakeOption.TimeZone(0);
+
+		// for debug: System.out.println("New connection object. Received properties are: " + props.toString());
 		// get supported property values from the props argument.
 		// When a value is found add it to the internal conn_props list for use by getClientInfo().
 		this.hostname = props.getProperty("host");
@@ -211,10 +215,10 @@ public class MonetConnection
 			conn_props.setProperty("hash", hash);
 
 		String autocommit_prop = props.getProperty("autocommit");
-		boolean initial_autocommit = true;
 		if (autocommit_prop != null) {
-			initial_autocommit = Boolean.parseBoolean(autocommit_prop);
-			conn_props.setProperty("autocommit", Boolean.toString(initial_autocommit));
+			boolean ac = Boolean.parseBoolean(autocommit_prop);
+			autoCommitSetting.set(ac);
+			conn_props.setProperty("autocommit", Boolean.toString(ac));
 		}
 
 		final String fetchsize_prop = props.getProperty("fetchsize");
@@ -222,7 +226,7 @@ public class MonetConnection
 			try {
 				int fetchsize = Integer.parseInt(fetchsize_prop);
 				if (fetchsize > 0 || fetchsize == -1) {
-					this.defaultFetchSize = fetchsize;
+					replySizeSetting.set(fetchsize);
 					conn_props.setProperty("fetchsize", fetchsize_prop);
 				} else {
 					addWarning("Fetch size must either be positive or -1. Value " + fetchsize + " ignored", "M1M05");
@@ -291,16 +295,18 @@ public class MonetConnection
 			server.setDatabase(database);
 		server.setLanguage(language);
 
+		// calculate our time zone offset
 		final Calendar cal = Calendar.getInstance();
 		int offsetMillis = cal.get(Calendar.ZONE_OFFSET) + cal.get(Calendar.DST_OFFSET);
 		int offsetSeconds = offsetMillis / 1000;
-		final HandshakeOptions handshakeOptions = new HandshakeOptions();
-		handshakeOptions.set(Setting.AutoCommit, initial_autocommit ? 1 : 0);
-		handshakeOptions.set(Setting.TimeZone, offsetSeconds);
-		handshakeOptions.set(Setting.ReplySize, defaultFetchSize);
-//		handshakeOptions.set(Setting.SizeHeader, 1);
-		server.setHandshakeOptions(handshakeOptions);
-		autoCommit = initial_autocommit;
+		timeZoneSetting.set(offsetSeconds);
+
+		server.setHandshakeOptions(new HandshakeOption[] {
+				autoCommitSetting,
+				replySizeSetting,
+				sizeHeaderSetting,
+				timeZoneSetting,
+		});
 
 		// we're debugging here... uhm, should be off in real life
 		if (debug) {
@@ -381,50 +387,21 @@ public class MonetConnection
 			lang = LANG_UNKNOWN;
 		}
 
-		// The reply size is checked before every query and adjusted if
-		// necessary. Update our current belief of what the server is set to.
-		if (handshakeOptions.wasSentInHandshake(HandshakeOptions.Setting.ReplySize)) {
-			this.curReplySize = handshakeOptions.get(HandshakeOptions.Setting.ReplySize);
+		// Now take care of any handshake options not handled during the handshake
+		if (replySizeSetting.isSent()) {
+			this.curReplySize = replySizeSetting.get();
 		}
-
-		for (Setting setting : new Setting[] { Setting.SizeHeader }) {
-			if (handshakeOptions.mustSend(setting)) {
-				Integer value = handshakeOptions.get(setting); // guaranteed by mustSend to be non-null
-				String command = String.format("%s %d", setting.getXCommand(), value);
-				sendControlCommand(command);
-			}
-		}
-
-		// the following initialisers are only valid when the language is SQL...
+		this.defaultFetchSize = replySizeSetting.get();
 		if (lang == LANG_SQL) {
-			if (handshakeOptions.mustSend(Setting.AutoCommit)) {
-				setAutoCommit(handshakeOptions.get(Setting.AutoCommit) != 0);
+			if (autoCommitSetting.mustSend(autoCommit)) {
+				setAutoCommit(autoCommitSetting.get());
 			}
-
-			// set our time zone on the server, if we haven't already
-			if (handshakeOptions.mustSend(Setting.TimeZone)) {
-				final StringBuilder tz = new StringBuilder(64);
-				tz.append("SET TIME ZONE INTERVAL '");
-				int offsetMinutes = handshakeOptions.get(Setting.TimeZone) / 60;
-				if (offsetMinutes < 0) {
-					tz.append('-');
-					offsetMinutes = -offsetMinutes; // make it positive
-				} else {
-					tz.append('+');
-				}
-				int offsetHours = offsetMinutes / 60;
-				if (offsetHours < 10)
-					tz.append('0');
-				tz.append(offsetHours).append(':');
-				offsetMinutes -= offsetHours * 60;
-				if (offsetMinutes < 10)
-					tz.append('0');
-				tz.append(offsetMinutes).append("' HOUR TO MINUTE");
-				sendIndependentCommand(tz.toString());
+			if (sizeHeaderSetting.mustSend(false)) {
+				sendControlCommand("sizeheader 1");
 			}
-
-			// set sizeheader to 1 to enable sending "typesizes" info by the server (see mapi_set_size_header() in mapi.c)
-			sendControlCommand("sizeheader 1");
+			if (timeZoneSetting.mustSend(0)) {
+				setTimezone(timeZoneSetting.get());
+			}
 		}
 
 		// we're absolutely not closed, since we're brand new
@@ -1739,6 +1716,27 @@ public class MonetConnection
 		return downloadHandler;
 	}
 
+	public void setTimezone(int offsetSeconds) throws SQLException {
+		final StringBuilder tz = new StringBuilder(64);
+		tz.append("SET TIME ZONE INTERVAL '");
+		int offsetMinutes = offsetSeconds / 60;
+		if (offsetMinutes < 0) {
+			tz.append('-');
+			offsetMinutes = -offsetMinutes; // make it positive
+		} else {
+			tz.append('+');
+		}
+		int offsetHours = offsetMinutes / 60;
+		if (offsetHours < 10)
+			tz.append('0');
+		tz.append(offsetHours).append(':');
+		offsetMinutes -= offsetHours * 60;
+		if (offsetMinutes < 10)
+			tz.append('0');
+		tz.append(offsetMinutes).append("' HOUR TO MINUTE");
+		sendIndependentCommand(tz.toString());
+	}
+
 	/**
 	 * Local helper method to test whether the Connection object is closed
 	 * When closed it throws an SQLException
new file mode 100644
--- /dev/null
+++ b/src/main/java/org/monetdb/mcl/net/HandshakeOption.java
@@ -0,0 +1,100 @@
+package org.monetdb.mcl.net;
+
+public abstract class HandshakeOption<T> {
+	protected final int level;
+	protected final String handshakeField;
+	boolean sent = false;
+	T desiredValue;
+
+	protected HandshakeOption(int level, String handshakeField, T desiredValue) {
+		if (desiredValue == null) {
+			throw new IllegalArgumentException("initial value must not be null");
+		}
+		this.level = level;
+		this.handshakeField = handshakeField;
+		this.desiredValue = desiredValue;
+	}
+
+	public void set(T newValue) {
+		if (newValue == null) {
+			throw new IllegalArgumentException("new value must not be null");
+		}
+		desiredValue = newValue;
+	}
+
+	public T get() {
+		return desiredValue;
+	}
+
+	public int getLevel() {
+		return level;
+	}
+
+	public String getFieldName() {
+		return handshakeField;
+	}
+
+	public boolean isSent() {
+		return sent;
+	}
+
+	public void setSent(boolean b) {
+		sent = b;
+	}
+
+	public boolean mustSend(T currentValue) {
+		if (sent)
+			return false;
+		if (currentValue.equals(desiredValue))
+			return false;
+		return true;
+	}
+
+	abstract long numericValue();
+
+	protected static class BooleanOption extends HandshakeOption<Boolean> {
+		protected BooleanOption(int level, String name, Boolean initialValue) {
+			super(level, name, initialValue);
+		}
+
+		@Override
+		long numericValue() {
+			return desiredValue ? 1 : 0;
+		}
+	}
+
+	public static class AutoCommit extends BooleanOption {
+		public AutoCommit(boolean autoCommit) {
+			super(1, "auto_commit",  autoCommit);
+		}
+	}
+
+	public static class ReplySize extends HandshakeOption<Integer> {
+		public ReplySize(int size) {
+			super(2, "reply_size", size);
+		}
+
+		@Override
+		long numericValue() {
+			return desiredValue;
+		}
+	}
+
+	public static class SizeHeader extends BooleanOption {
+		public SizeHeader(boolean sendHeader) {
+			super(3, "size_header", sendHeader);
+			set(sendHeader);
+		}
+	}
+
+	public static class TimeZone extends HandshakeOption<Integer> {
+		public TimeZone(int offset) {
+			super(5, "time_zone", offset);
+		}
+
+		@Override
+		long numericValue() {
+			return desiredValue;
+		}
+	}
+}
deleted file mode 100644
--- a/src/main/java/org/monetdb/mcl/net/HandshakeOptions.java
+++ /dev/null
@@ -1,103 +0,0 @@
-/*
- * This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0.  If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/.
- *
- * Copyright 1997 - July 2008 CWI, August 2008 - 2022 MonetDB B.V.
- */
-
-package org.monetdb.mcl.net;
-
-import java.util.HashMap;
-import java.util.Map;
-
-/** Keep track of MAPI handshake options.
- *
- * Recent server versions (from 2021) allow you to send configuration information during
- * the authentication handshake so no additional round trips are necessary
- * when that has completed.
- *
- * This class keeps track of the values themselves, and also of whether or not they should still be sent.
- */
-final public class HandshakeOptions {
-	HashMap<Setting,Integer> options = new HashMap<>();
-	int handshakeLevel = 0;
-
-	public void set(Setting setting, int value) {
-		options.put(setting, value);
-	}
-
-	public Integer get(Setting setting) {
-		return options.get(setting);
-	}
-
-	public boolean wasSentInHandshake(Setting setting) {
-		return setting.isSupported(this.handshakeLevel);
-	}
-
-	public boolean mustSend(Setting setting) {
-		if (wasSentInHandshake(setting)) {
-			return false;
-		}
-		Integer value = options.get(setting);
-		return value != null && value != setting.defaultValue;
-	}
-
-	public String formatHandshakeResponse(int serverLevel) {
-		StringBuilder opts = new StringBuilder(100);
-
-		for (Map.Entry<Setting, Integer> entry: options.entrySet()) {
-			Setting setting = entry.getKey();
-			Integer value = entry.getValue();
-			if (setting.isSupported(serverLevel)) {
-				if (opts.length() > 0) {
-					opts.append(",");
-				}
-				opts.append(setting.field);
-				opts.append("=");
-				opts.append(value);
-			}
-		}
-
-		this.handshakeLevel = serverLevel;
-
-		return opts.toString();
-	}
-
-	public enum Setting {
-		AutoCommit("auto_commit", 1, 1),
-		ReplySize("reply_size", 2, 100),
-		SizeHeader("size_header", "sizeheader", 3, 0),
-		// ColumnarProtocol("columnar_protocol", 4),
-		TimeZone("time_zone", 5, 0),
-		;
-
-		private final int level;
-		private final String field;
-		private final String xcommand;
-		private final int defaultValue;
-
-		Setting(String field, int level, int defaultValue) {
-			this(field, field, level, defaultValue);
-		}
-
-		Setting(String field, String xcommand, int level, int defaultValue) {
-			this.field = field;
-			this.xcommand = xcommand;
-			this.level = level;
-			this.defaultValue = defaultValue;
-		}
-
-		public boolean isSupported(int serverLevel) {
-			return this.level < serverLevel;
-		}
-
-		public String getXCommand() {
-			return xcommand;
-		}
-
-		public Integer getDefaultValue() {
-			return defaultValue;
-		}
-	}
-}
--- a/src/main/java/org/monetdb/mcl/net/MapiSocket.java
+++ b/src/main/java/org/monetdb/mcl/net/MapiSocket.java
@@ -125,7 +125,7 @@ public class MapiSocket {	/* cannot (yet
 	private final byte[] blklen = new byte[2];
 
 	/** Options that can be sent during the auth handshake if the server supports it */
-	private HandshakeOptions handshakeOptions;
+	private HandshakeOption[] handshakeOptions;
 
 	/**
 	 * Constructs a new MapiSocket.
@@ -562,7 +562,21 @@ public class MapiSocket {	/* cannot (yet
 							} catch (NumberFormatException e) {
 								throw new MCLParseException("Invalid handshake level: " + chaltok[6]);
 							}
-							response += handshakeOptions.formatHandshakeResponse(level);
+							boolean first = true;
+							for (HandshakeOption opt: handshakeOptions) {
+								if (opt.getLevel() < level) {
+									// server supports it
+									if (first) {
+										first = false;
+									} else {
+										response += ",";
+									}
+									String key = opt.getFieldName();
+									long value = opt.numericValue();
+									response += opt.getFieldName() + "=" + value;
+									opt.setSent(true);
+								}
+							}
 							break;
 						}
 					}
@@ -718,14 +732,10 @@ public class MapiSocket {	/* cannot (yet
 			log.flush();
 	}
 
-	public void setHandshakeOptions(HandshakeOptions handshakeOptions) {
+	public void setHandshakeOptions(HandshakeOption[] handshakeOptions) {
 		this.handshakeOptions = handshakeOptions;
 	}
 
-	public HandshakeOptions getHandshakeOptions() {
-		return handshakeOptions;
-	}
-
 	/**
 	 * For internal use
 	 *