From bb978b4028b2420d08a3770898d458d04b1c63af Mon Sep 17 00:00:00 2001 From: "Brian J. Cardiff" Date: Fri, 20 Dec 2024 20:11:45 -0300 Subject: [PATCH 1/6] Add connection options specs --- spec/connection_options_spec.cr | 61 +++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 spec/connection_options_spec.cr diff --git a/spec/connection_options_spec.cr b/spec/connection_options_spec.cr new file mode 100644 index 0000000..11388d8 --- /dev/null +++ b/spec/connection_options_spec.cr @@ -0,0 +1,61 @@ +require "./spec_helper" + +private def from_uri(uri) + Connection::Options.from_uri(URI.parse(uri)) +end + +describe Connection::Options do + describe ".from_uri" do + it "parses mysql://user@host/db" do + from_uri("mysql://root@localhost/test").should eq( + MySql::Connection::Options.new( + host: "localhost", + port: 3306, + username: "root", + password: nil, + initial_catalog: "test", + charset: Collations.default_collation + ) + ) + end + + it "parses mysql://host" do + from_uri("mysql://localhost").should eq( + MySql::Connection::Options.new( + host: "localhost", + port: 3306, + username: nil, + password: nil, + initial_catalog: nil, + charset: Collations.default_collation + ) + ) + end + + it "parses mysql://host:port" do + from_uri("mysql://localhost:1234").should eq( + MySql::Connection::Options.new( + host: "localhost", + port: 1234, + username: nil, + password: nil, + initial_catalog: nil, + charset: Collations.default_collation + ) + ) + end + + it "parses ?encoding=..." do + from_uri("mysql://localhost:1234?encoding=utf8mb4_unicode_520_ci").should eq( + MySql::Connection::Options.new( + host: "localhost", + port: 1234, + username: nil, + password: nil, + initial_catalog: nil, + charset: "utf8mb4_unicode_520_ci" + ) + ) + end + end +end From 584b371a12640331237f49865b0efbc8217a119a Mon Sep 17 00:00:00 2001 From: "Brian J. Cardiff" Date: Fri, 20 Dec 2024 23:11:59 -0300 Subject: [PATCH 2/6] Add specs for unix socket transport --- spec/connection_options_spec.cr | 80 +++++++++++++++++++++++++++++---- src/mysql/connection.cr | 45 +++++++++++++------ 2 files changed, 103 insertions(+), 22 deletions(-) diff --git a/spec/connection_options_spec.cr b/spec/connection_options_spec.cr index 11388d8..baab0a1 100644 --- a/spec/connection_options_spec.cr +++ b/spec/connection_options_spec.cr @@ -4,13 +4,20 @@ private def from_uri(uri) Connection::Options.from_uri(URI.parse(uri)) end +private def tcp(host, port) + MySql::Connection::TCPSocketTransport.new(host: host, port: port) +end + +private def socket(path) + MySql::Connection::UnixSocketTransport.new(path: Path.new(path)) +end + describe Connection::Options do describe ".from_uri" do it "parses mysql://user@host/db" do from_uri("mysql://root@localhost/test").should eq( MySql::Connection::Options.new( - host: "localhost", - port: 3306, + transport: tcp("localhost", 3306), username: "root", password: nil, initial_catalog: "test", @@ -22,8 +29,7 @@ describe Connection::Options do it "parses mysql://host" do from_uri("mysql://localhost").should eq( MySql::Connection::Options.new( - host: "localhost", - port: 3306, + transport: tcp("localhost", 3306), username: nil, password: nil, initial_catalog: nil, @@ -35,8 +41,7 @@ describe Connection::Options do it "parses mysql://host:port" do from_uri("mysql://localhost:1234").should eq( MySql::Connection::Options.new( - host: "localhost", - port: 1234, + transport: tcp("localhost", 1234), username: nil, password: nil, initial_catalog: nil, @@ -48,8 +53,55 @@ describe Connection::Options do it "parses ?encoding=..." do from_uri("mysql://localhost:1234?encoding=utf8mb4_unicode_520_ci").should eq( MySql::Connection::Options.new( - host: "localhost", - port: 1234, + transport: tcp("localhost", 1234), + username: nil, + password: nil, + initial_catalog: nil, + charset: "utf8mb4_unicode_520_ci" + ) + ) + end + + it "parses mysql://user@host?database=db" do + from_uri("mysql://root@localhost?database=test").should eq( + MySql::Connection::Options.new( + transport: tcp("localhost", 3306), + username: "root", + password: nil, + initial_catalog: "test", + charset: Collations.default_collation + ) + ) + end + + it "parses mysql:///path/to/socket" do + from_uri("mysql:///path/to/socket").should eq( + MySql::Connection::Options.new( + transport: socket("/path/to/socket"), + username: nil, + password: nil, + initial_catalog: nil, + charset: Collations.default_collation + ) + ) + end + + it "parses mysql:///path/to/socket?database=test" do + from_uri("mysql:///path/to/socket?database=test").should eq( + MySql::Connection::Options.new( + transport: socket("/path/to/socket"), + username: nil, + password: nil, + initial_catalog: "test", + charset: Collations.default_collation + ) + ) + end + + it "parses mysql:///path/to/socket?encoding=utf8mb4_unicode_520_ci" do + from_uri("mysql:///path/to/socket?encoding=utf8mb4_unicode_520_ci").should eq( + MySql::Connection::Options.new( + transport: socket("/path/to/socket"), username: nil, password: nil, initial_catalog: nil, @@ -57,5 +109,17 @@ describe Connection::Options do ) ) end + + it "parses mysql://user:pass@/path/to/socket?database=test" do + from_uri("mysql://root:password@/path/to/socket?database=test").should eq( + MySql::Connection::Options.new( + transport: socket("/path/to/socket"), + username: "root", + password: "password", + initial_catalog: "test", + charset: Collations.default_collation + ) + ) + end end end diff --git a/src/mysql/connection.cr b/src/mysql/connection.cr index bc648d2..210e841 100644 --- a/src/mysql/connection.cr +++ b/src/mysql/connection.cr @@ -1,30 +1,40 @@ require "socket" class MySql::Connection < DB::Connection + record UnixSocketTransport, path : Path + record TCPSocketTransport, host : String, port : Int32 + record Options, - host : String, - port : Int32, + transport : UnixSocketTransport | TCPSocketTransport, username : String?, password : String?, initial_catalog : String?, charset : String do def self.from_uri(uri : URI) : Options - host = uri.hostname || raise "no host provided" - port = uri.port || 3306 + params = uri.query_params + initial_catalog = params["database"]? + + if (host = uri.hostname) && !host.blank? + port = uri.port || 3306 + transport = TCPSocketTransport.new(host: host, port: port) + + # for tcp socket we support the first component to be the database + # but the query string takes presedence because it's more explicit + if initial_catalog.nil? && (path = uri.path) && path.size > 1 + initial_catalog = path[1..-1] + end + else + transport = UnixSocketTransport.new(path: Path.new(uri.path)) + end + username = uri.user password = uri.password - charset = uri.query_params.fetch "encoding", Collations.default_collation - - path = uri.path - if path && path.size > 1 - initial_catalog = path[1..-1] - else - initial_catalog = nil - end + charset = params.fetch "encoding", Collations.default_collation Options.new( - host: host, port: port, username: username, password: password, + transport: transport, + username: username, password: password, initial_catalog: initial_catalog, charset: charset) end end @@ -36,7 +46,14 @@ class MySql::Connection < DB::Connection begin charset_id = Collations.id_for_collation(mysql_options.charset).to_u8 - @socket = TCPSocket.new(mysql_options.host, mysql_options.port) + @socket = + case transport = mysql_options.transport + when TCPSocketTransport + TCPSocket.new(mysql_options.host, mysql_options.port) + when UnixSocketTransport + raise "not implemented" + end + handshake = read_packet(Protocol::HandshakeV10) write_packet(1) do |packet| From e1947d8d6dddac1dcccd0c80d38bd383ea286375 Mon Sep 17 00:00:00 2001 From: "Brian J. Cardiff" Date: Wed, 25 Dec 2024 19:10:15 -0300 Subject: [PATCH 3/6] Add UnixSocket support to connection --- src/mysql/connection.cr | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/mysql/connection.cr b/src/mysql/connection.cr index 210e841..c423499 100644 --- a/src/mysql/connection.cr +++ b/src/mysql/connection.cr @@ -24,7 +24,8 @@ class MySql::Connection < DB::Connection initial_catalog = path[1..-1] end else - transport = UnixSocketTransport.new(path: Path.new(uri.path)) + # uri.path has a final / we want to drop + transport = UnixSocketTransport.new(path: Path.new(uri.path.chomp('/'))) end username = uri.user @@ -41,17 +42,17 @@ class MySql::Connection < DB::Connection def initialize(options : ::DB::Connection::Options, mysql_options : ::MySql::Connection::Options) super(options) - @socket = uninitialized TCPSocket + @socket = uninitialized UNIXSocket | TCPSocket begin charset_id = Collations.id_for_collation(mysql_options.charset).to_u8 @socket = case transport = mysql_options.transport - when TCPSocketTransport - TCPSocket.new(mysql_options.host, mysql_options.port) - when UnixSocketTransport - raise "not implemented" + in TCPSocketTransport + TCPSocket.new(transport.host, transport.port) + in UnixSocketTransport + UNIXSocket.new(transport.path.to_s) end handshake = read_packet(Protocol::HandshakeV10) From 1401fcd3faa6a2c77e7c004341e083e384cfc811 Mon Sep 17 00:00:00 2001 From: "Brian J. Cardiff" Date: Wed, 25 Dec 2024 19:40:56 -0300 Subject: [PATCH 4/6] Update specs to be able to run with socket --- spec/driver_spec.cr | 6 +++--- spec/spec_helper.cr | 8 ++++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/spec/driver_spec.cr b/spec/driver_spec.cr index 0a13ae3..a96ef7f 100644 --- a/spec/driver_spec.cr +++ b/spec/driver_spec.cr @@ -32,7 +32,7 @@ describe Driver do db.exec "FLUSH PRIVILEGES" end - DB.open "mysql://crystal_test:secret@#{database_host}/crystal_mysql_test" do |db| + DB.open "mysql://crystal_test:secret@#{database_host}?database=crystal_mysql_test" do |db| db.scalar("SELECT DATABASE()").should eq("crystal_mysql_test") db.scalar("SELECT CURRENT_USER()").should match(/^crystal_test@/) end @@ -48,7 +48,7 @@ describe Driver do db.exec "CREATE DATABASE crystal_mysql_test" # By default, the encoding for the DB connection is set to utf8_general_ci - DB.open "mysql://crystal_test:secret@#{database_host}/crystal_mysql_test" do |db| + DB.open "mysql://crystal_test:secret@#{database_host}?database=crystal_mysql_test" do |db| db.scalar("SELECT @@collation_connection").should eq("utf8_general_ci") db.scalar("SELECT @@character_set_connection").should eq("utf8") end @@ -61,7 +61,7 @@ describe Driver do db.exec "DROP DATABASE IF EXISTS crystal_mysql_test" db.exec "CREATE DATABASE crystal_mysql_test" - DB.open "mysql://crystal_test:secret@#{database_host}/crystal_mysql_test?encoding=utf8mb4_unicode_520_ci" do |db| + DB.open "mysql://crystal_test:secret@#{database_host}?database=crystal_mysql_test&encoding=utf8mb4_unicode_520_ci" do |db| db.scalar("SELECT @@collation_connection").should eq("utf8mb4_unicode_520_ci") db.scalar("SELECT @@character_set_connection").should eq("utf8mb4") end diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr index 8493f18..62bb5ae 100644 --- a/spec/spec_helper.cr +++ b/spec/spec_helper.cr @@ -5,7 +5,11 @@ require "semantic_version" include MySql def db_url(initial_db = nil) - "mysql://root@#{database_host}/#{initial_db}" + if initial_db + "mysql://root@#{database_host}?database=#{initial_db}" + else + "mysql://root@#{database_host}" + end end def database_host @@ -18,7 +22,7 @@ def with_db(database_name, options = nil, &block : DB::Database ->) db.exec "CREATE DATABASE crystal_mysql_test" end - DB.open "#{db_url(database_name)}?#{options}", &block + DB.open "#{db_url(database_name)}&#{options}", &block ensure DB.open db_url do |db| db.exec "DROP DATABASE IF EXISTS crystal_mysql_test" From 3289dc95a438399ba0b35990ba11c1cd6181cfa3 Mon Sep 17 00:00:00 2001 From: "Brian J. Cardiff" Date: Wed, 25 Dec 2024 20:10:12 -0300 Subject: [PATCH 5/6] Use native mysql in CI --- .github/workflows/ci.yml | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fd52a57..ab450f6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,7 +14,12 @@ jobs: matrix: os: [ubuntu-latest] crystal: [1.3.0, latest, nightly] - mysql_docker_image: ["mysql:5.6", "mysql:5.7"] + mysql_version: ["5.6", "5.7"] + database_host: ["default", "/tmp/mysql.sock"] + exclude: + # mysql 5.6 fails to run some specs using socket + - mysql_version: 5.6 + database_host: /tmp/mysql.sock runs-on: ${{ matrix.os }} steps: - name: Install Crystal @@ -22,23 +27,28 @@ jobs: with: crystal: ${{ matrix.crystal }} - - name: Shutdown Ubuntu MySQL (SUDO) - run: sudo service mysql stop # Shutdown the Default MySQL, "sudo" is necessary, please not remove it + - id: setup-mysql + uses: shogo82148/actions-setup-mysql@v1 + with: + mysql-version: ${{ matrix.mysql_version }} - - name: Setup MySQL + - name: Wait for MySQL run: | - docker run -e MYSQL_ALLOW_EMPTY_PASSWORD=yes -p 3306:3306 -d ${{ matrix.mysql_docker_image }} - docker ps -a # log docker image while ! echo exit | nc localhost 3306; do sleep 5; done # wait mysql to start accepting connections - name: Download source - uses: actions/checkout@v2 + uses: actions/checkout@v4 - name: Install shards run: shards install - - name: Run specs + - name: Run specs (Socket) + run: DATABASE_HOST=${{ steps.setup-mysql.outputs.base-dir }}/tmp/mysql.sock crystal spec + if: matrix.database_host == '/tmp/mysql.sock' + + - name: Run specs (Plain TCP) run: crystal spec + if: matrix.database_host == 'default' - name: Check formatting run: crystal tool format; git diff --exit-code From 946c7cf56e1088480c6021042b89d844325bf773 Mon Sep 17 00:00:00 2001 From: "Brian J. Cardiff" Date: Wed, 25 Dec 2024 20:21:05 -0300 Subject: [PATCH 6/6] Drop mysql 5.6 in CI --- .github/workflows/ci.yml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ab450f6..8265eda 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,12 +14,8 @@ jobs: matrix: os: [ubuntu-latest] crystal: [1.3.0, latest, nightly] - mysql_version: ["5.6", "5.7"] + mysql_version: ["5.7"] database_host: ["default", "/tmp/mysql.sock"] - exclude: - # mysql 5.6 fails to run some specs using socket - - mysql_version: 5.6 - database_host: /tmp/mysql.sock runs-on: ${{ matrix.os }} steps: - name: Install Crystal