Add :sid and :service_name connection options

README.md

@@ -120,21 +120,27 @@ plsql.activerecord_class = ActiveRecord::Base
 and then you do not need to specify plsql.connection (this is also safer when ActiveRecord reestablishes connection to database).
 
 
-### JRuby JDBC connection:
+### Connection options: `:database`, `:service_name`, `:sid`
 
-When using JRuby, the `connect!` method with `:host` and `:database` options uses the thin-style service name syntax by default:
+`connect!` accepts three mutually-exclusive ways to identify the target Oracle instance, matching the [oracle-enhanced adapter](https://github.com/rsim/oracle-enhanced):
 
 ```ruby
-# Connects using service name syntax: jdbc:oracle:thin:@//localhost:1521/MYSERVICENAME
+# By service name (recommended for 12c+ / PDBs)
+plsql.connect! username: "hr", password: "hr", host: "localhost", service_name: "MYSERVICENAME"
+
+# By SID (legacy single-instance, e.g. Oracle 11g XE)
+plsql.connect! username: "hr", password: "hr", host: "localhost", sid: "MYSID"
+
+# `:database` is still accepted and is treated as a service name
 plsql.connect! username: "hr", password: "hr", host: "localhost", database: "MYSERVICENAME"
 ```
 
-If you need to connect using the legacy SID syntax (for Oracle databases older than 12c), prefix the database name with a colon:
+Supplying more than one of `:database`, `:service_name`, `:sid` raises `ArgumentError`. Both `:service_name` and `:sid` work under the OCI driver (MRI) and the JDBC driver (JRuby). Under JRuby:
 
-```ruby
-# Connects using SID syntax: jdbc:oracle:thin:@localhost:1521:MYSID
-plsql.connect! username: "hr", password: "hr", host: "localhost", database: ":MYSID"
-```
+* `:service_name` builds `jdbc:oracle:thin:@//host:port/service_name`
+* `:sid` builds `jdbc:oracle:thin:@host:port:SID`
+
+The legacy `database: ":MYSID"` colon-prefix overload still works for one release but is deprecated; use `sid: "MYSID"` instead.
 
 ### Cheat Sheet:
 

lib/plsql/connection.rb

@@ -3,6 +3,48 @@ class Connection
     attr_reader :raw_driver
     attr_reader :activerecord_class
 
+    # `:database` is the primary option and is treated as a service name.
+    # `:service_name` is provided for compatibility with the
+    # oracle-enhanced adapter (rsim/oracle-enhanced#2669) and is treated
+    # as an alias of `:database`. `:sid` selects the legacy SID URL form
+    # for single-instance Oracle deployments (e.g. 11g XE), and exists to
+    # replace the deprecated `database: ":SID"` colon-prefix entry. The
+    # three options are mutually exclusive.
+    #
+    # SID character set matches the oracle-enhanced adapter: alphanumeric,
+    # underscore, `$`, `#`. No length cap — INSTANCE_NAME allows up to 255
+    # characters in 19c+; the historical 8-char limit applies to DB_NAME,
+    # not to the SID/INSTANCE_NAME the listener registers under.
+    SID_IDENTIFIER_PATTERN = /\A[\w$#]+\z/
+
+    # Validates :database / :service_name / :sid in `params` and folds
+    # :service_name into :database so downstream URL builders only need to
+    # branch on :database vs :sid. Raises ArgumentError on conflicts or
+    # invalid values.
+    def self.resolve_database_aliases!(params)
+      provided_keys = []
+      provided_keys << ":database"     if params[:database]
+      provided_keys << ":service_name" if params[:service_name]
+      provided_keys << ":sid"          if params[:sid]
+      if provided_keys.size > 1
+        raise ArgumentError,
+          "Cannot specify more than one of #{provided_keys.join(', ')}; they are mutually exclusive."
+      end
+
+      if (svc = params[:service_name])
+        if svc.to_s.start_with?("/")
+          raise ArgumentError,
+            "Invalid :service_name value #{svc.inspect}; must not start with '/'."
+        end
+        params[:database] = svc
+      end
+
+      if (sid = params[:sid]) && !sid.to_s.match?(SID_IDENTIFIER_PATTERN)
+        raise ArgumentError,
+          "Invalid :sid value #{sid.inspect}; must be an Oracle SID (alphanumeric, underscore, $, #)."
+      end
+    end
+
     def initialize(raw_conn, ar_class = nil) # :nodoc:
       @raw_driver = self.class.driver_type
       @raw_connection = raw_conn

lib/plsql/jdbc_connection.rb

@@ -76,6 +76,14 @@ def self.create_raw(params)
     end
 
     def self.jdbc_connection_url(params)
+      Connection.resolve_database_aliases!(params)
+
+      if (sid = params[:sid])
+        host = params[:host] || "localhost"
+        port = params[:port] || 1521
+        return "jdbc:oracle:thin:@#{host}:#{port}:#{sid}"
+      end
+
       database = params[:database]
       if ENV["TNS_ADMIN"] && database && database !~ %r{\A[:/]} && !params[:host] && !params[:url]
         "jdbc:oracle:thin:@#{database}"
@@ -88,6 +96,7 @@ def self.jdbc_connection_url(params)
         port = params[:port] || 1521
 
         if database =~ /^:/
+          warn "[ruby-plsql] database: \":...\" (SID via colon prefix) is deprecated; use sid: \"...\" instead"
           # SID syntax: jdbc:oracle:thin:@host:port:SID
           "jdbc:oracle:thin:@#{host}:#{port}#{database}"
         else

lib/plsql/oci_connection.rb

@@ -22,7 +22,15 @@
 module PLSQL
   class OCIConnection < Connection # :nodoc:
     def self.create_raw(params)
-      connection_string = if params[:host]
+      Connection.resolve_database_aliases!(params)
+
+      connection_string = if (sid = params[:sid])
+        # OCI has no SID form for EZCONNECT; build a TNS connect descriptor
+        # so :sid works without requiring a tnsnames.ora entry.
+        host = params[:host] || "localhost"
+        port = params[:port] || 1521
+        "(DESCRIPTION=(ADDRESS=(PROTOCOL=TCP)(HOST=#{host})(PORT=#{port}))(CONNECT_DATA=(SID=#{sid})))"
+      elsif params[:host]
         "//#{params[:host]}:#{params[:port] || 1521}/#{params[:database]}"
       else
         params[:database]

spec/plsql/connection_spec.rb

@@ -483,9 +483,11 @@
       end
     end
 
-    it "should use SID syntax when database starts with colon" do
-      url = PLSQL::JDBCConnection.jdbc_connection_url(host: "myhost", port: 1521, database: ":MYSID")
-      expect(url).to eq "jdbc:oracle:thin:@myhost:1521:MYSID"
+    it "should use SID syntax when database starts with colon (deprecated)" do
+      expect {
+        url = PLSQL::JDBCConnection.jdbc_connection_url(host: "myhost", port: 1521, database: ":MYSID")
+        expect(url).to eq "jdbc:oracle:thin:@myhost:1521:MYSID"
+      }.to output(/deprecated/).to_stderr
     end
 
     it "should use service name syntax when database starts with slash" do
@@ -515,12 +517,14 @@
       end
     end
 
-    it "should use SID syntax when TNS_ADMIN is set and database starts with colon" do
+    it "should use SID syntax when TNS_ADMIN is set and database starts with colon (deprecated)" do
       original_tns_admin = ENV["TNS_ADMIN"]
       ENV["TNS_ADMIN"] = "/path/to/tns"
       begin
-        url = PLSQL::JDBCConnection.jdbc_connection_url(database: ":MYSID")
-        expect(url).to eq "jdbc:oracle:thin:@localhost:1521:MYSID"
+        expect {
+          url = PLSQL::JDBCConnection.jdbc_connection_url(database: ":MYSID")
+          expect(url).to eq "jdbc:oracle:thin:@localhost:1521:MYSID"
+        }.to output(/deprecated/).to_stderr
       ensure
         ENV["TNS_ADMIN"] = original_tns_admin
       end
@@ -537,8 +541,139 @@
       url = PLSQL::JDBCConnection.jdbc_connection_url(host: "myhost", database: "MYSERVICENAME", url: custom_url)
       expect(url).to eq custom_url
     end
+
+    context ":sid option" do
+      it "builds SID URL form" do
+        url = PLSQL::JDBCConnection.jdbc_connection_url(host: "myhost", port: 1521, sid: "MYSID")
+        expect(url).to eq "jdbc:oracle:thin:@myhost:1521:MYSID"
+      end
+
+      it "defaults host and port when not specified" do
+        url = PLSQL::JDBCConnection.jdbc_connection_url(sid: "MYSID")
+        expect(url).to eq "jdbc:oracle:thin:@localhost:1521:MYSID"
+      end
+
+      it "rejects values starting with '/'" do
+        expect {
+          PLSQL::JDBCConnection.jdbc_connection_url(sid: "/MYSID")
+        }.to raise_error(ArgumentError, /Invalid :sid value/)
+      end
+
+      it "rejects values starting with ':'" do
+        expect {
+          PLSQL::JDBCConnection.jdbc_connection_url(sid: ":MYSID")
+        }.to raise_error(ArgumentError, /Invalid :sid value/)
+      end
+
+      it "rejects TNS connect descriptors" do
+        expect {
+          PLSQL::JDBCConnection.jdbc_connection_url(
+            sid: "(DESCRIPTION=(ADDRESS=(PROTOCOL=TCP)(HOST=foo)(PORT=1521))(CONNECT_DATA=(SID=XE)))"
+          )
+        }.to raise_error(ArgumentError, /Invalid :sid value/)
+      end
+    end
+
+    context ":service_name option" do
+      it "builds service-name URL form" do
+        url = PLSQL::JDBCConnection.jdbc_connection_url(host: "myhost", port: 1521, service_name: "MYSVC")
+        expect(url).to eq "jdbc:oracle:thin:@//myhost:1521/MYSVC"
+      end
+
+      it "rejects values starting with '/'" do
+        expect {
+          PLSQL::JDBCConnection.jdbc_connection_url(service_name: "/MYSVC")
+        }.to raise_error(ArgumentError, /Invalid :service_name value/)
+      end
+    end
+
+    context "mutual exclusion" do
+      it "raises when :database and :service_name are both set" do
+        expect {
+          PLSQL::JDBCConnection.jdbc_connection_url(database: "X", service_name: "Y")
+        }.to raise_error(ArgumentError, /Cannot specify more than one of :database, :service_name/)
+      end
+
+      it "raises when :database and :sid are both set" do
+        expect {
+          PLSQL::JDBCConnection.jdbc_connection_url(database: "X", sid: "Y")
+        }.to raise_error(ArgumentError, /Cannot specify more than one of :database, :sid/)
+      end
+
+      it "raises when :service_name and :sid are both set" do
+        expect {
+          PLSQL::JDBCConnection.jdbc_connection_url(service_name: "X", sid: "Y")
+        }.to raise_error(ArgumentError, /Cannot specify more than one of :service_name, :sid/)
+      end
+
+      it "raises when all three are set" do
+        expect {
+          PLSQL::JDBCConnection.jdbc_connection_url(database: "X", service_name: "Y", sid: "Z")
+        }.to raise_error(ArgumentError, /Cannot specify more than one of :database, :service_name, :sid/)
+      end
+    end
   end if defined?(JRuby)
 
+  describe "OCI connection string" do
+    def captured_connection_string(params)
+      captured = nil
+      stub_oci8 = Class.new do
+        define_singleton_method(:new) do |_user, _password, conn_str|
+          captured = conn_str
+          Object.new
+        end
+      end
+      stub_const("OCI8", stub_oci8)
+      PLSQL::OCIConnection.create_raw({ username: "u", password: "p" }.merge(params))
+      captured
+    end
+
+    context ":sid option" do
+      it "builds an inline TNS connect descriptor" do
+        conn_str = captured_connection_string(host: "myhost", port: 1521, sid: "MYSID")
+        expect(conn_str).to eq "(DESCRIPTION=(ADDRESS=(PROTOCOL=TCP)(HOST=myhost)(PORT=1521))(CONNECT_DATA=(SID=MYSID)))"
+      end
+
+      it "defaults host and port when not specified" do
+        conn_str = captured_connection_string(sid: "MYSID")
+        expect(conn_str).to eq "(DESCRIPTION=(ADDRESS=(PROTOCOL=TCP)(HOST=localhost)(PORT=1521))(CONNECT_DATA=(SID=MYSID)))"
+      end
+
+      it "rejects values starting with ':'" do
+        expect {
+          captured_connection_string(sid: ":MYSID")
+        }.to raise_error(ArgumentError, /Invalid :sid value/)
+      end
+    end
+
+    context ":service_name option" do
+      it "builds the EZCONNECT-style connection string" do
+        conn_str = captured_connection_string(host: "myhost", port: 1521, service_name: "MYSVC")
+        expect(conn_str).to eq "//myhost:1521/MYSVC"
+      end
+
+      it "rejects values starting with '/'" do
+        expect {
+          captured_connection_string(service_name: "/MYSVC")
+        }.to raise_error(ArgumentError, /Invalid :service_name value/)
+      end
+    end
+
+    context "mutual exclusion" do
+      it "raises when :database and :sid are both set" do
+        expect {
+          captured_connection_string(database: "X", sid: "Y")
+        }.to raise_error(ArgumentError, /Cannot specify more than one of :database, :sid/)
+      end
+
+      it "raises when :service_name and :sid are both set" do
+        expect {
+          captured_connection_string(service_name: "X", sid: "Y")
+        }.to raise_error(ArgumentError, /Cannot specify more than one of :service_name, :sid/)
+      end
+    end
+  end unless defined?(JRuby)
+
   describe "logoff" do
     before(:each) do
       # restore connection before each test