From 9aa86ea216b93ce2dd13ed2878d30496613e194e Mon Sep 17 00:00:00 2001 From: XLEED_DEV Date: Wed, 16 Jul 2025 16:10:10 -0500 Subject: [PATCH 1/7] New PostgreSQL shared storage implementation --- lib/bas/bot/base.rb | 9 +++++++- lib/bas/shared_storage/base.rb | 5 +++++ lib/bas/shared_storage/postgres.rb | 31 +++++++++++++++++++++------ lib/bas/utils/postgres/connection.rb | 32 ++++++++++++++++++++++++++++ 4 files changed, 69 insertions(+), 8 deletions(-) create mode 100644 lib/bas/utils/postgres/connection.rb diff --git a/lib/bas/bot/base.rb b/lib/bas/bot/base.rb index b958fbc..7a161fc 100644 --- a/lib/bas/bot/base.rb +++ b/lib/bas/bot/base.rb @@ -15,7 +15,7 @@ class Base attr_accessor :read_response, :process_response, :write_response def initialize(options, shared_storage_reader, shared_storage_writer = nil) - @process_options = options || {} + @process_options = options || { close_connections_after_process: true } @shared_storage_reader = shared_storage_reader @shared_storage_writer = shared_storage_writer || shared_storage_reader end @@ -31,6 +31,8 @@ def execute @shared_storage_reader.set_processed @write_response = write + + close_connections if @process_options[:close_connections_after_process].eql?(true) end protected @@ -60,6 +62,11 @@ def unprocessable_response read_data.nil? || read_data == {} || read_data.any? { |_key, value| [[], "", nil].include?(value) } end + + def close_connections + @shared_storage_reader.close_connections if @shared_storage_reader.respond_to?(:close_connections) + @shared_storage_writer.close_connections if @shared_storage_writer.respond_to?(:close_connections) + end end end end diff --git a/lib/bas/shared_storage/base.rb b/lib/bas/shared_storage/base.rb index 0592d1a..ddfe1d9 100644 --- a/lib/bas/shared_storage/base.rb +++ b/lib/bas/shared_storage/base.rb @@ -21,6 +21,11 @@ def set_in_process; end def set_processed; end + def close_connections + # TODO: Leave this method empty after testing + puts "Closing connection for #{self.class.name}" + end + protected def read diff --git a/lib/bas/shared_storage/postgres.rb b/lib/bas/shared_storage/postgres.rb index 625be8a..6644dc1 100644 --- a/lib/bas/shared_storage/postgres.rb +++ b/lib/bas/shared_storage/postgres.rb @@ -2,7 +2,7 @@ require_relative "base" require_relative "types/read" -require_relative "../utils/postgres/request" +require_relative "../utils/postgres/connection" require_relative "../version" require "json" @@ -17,17 +17,25 @@ class Postgres < Bas::SharedStorage::Base TABLE_PARAMS = "data, tag, archived, stage, status, error_message, version" def read - params = { connection: read_options[:connection], query: read_query } + establish_connection(:read) - first_result = Utils::Postgres::Request.execute(params).first || {} + first_result = @read_connection.query(read_query).first || {} @read_response = Bas::SharedStorage::Types::Read.new(first_result[:id], first_result[:data], first_result[:inserted_at]) end def write(data) - params = { connection: write_options[:connection], query: write_query(data) } - @write_response = Utils::Postgres::Request.execute(params) + establish_connection(:write) + + @write_response = @write_connection.query(write_query(data)) + end + + def close_connections + @read_connection&.finish + @write_connection&.finish + @read_connection = nil + @write_connection = nil end def set_in_process @@ -44,6 +52,15 @@ def set_processed private + def establish_connection(action) + case action + when :read + @read_connection ||= Utils::Postgres::Connection.new(read_options[:connection]) + when :write + @write_connection ||= Utils::Postgres::Connection.new(write_options[:connection]) + end + end + def read_query query = "SELECT id, data, inserted_at FROM #{read_options[:db_table]} WHERE status='success' AND #{where}" @@ -78,9 +95,9 @@ def write_params(data) end def update_stage(id, stage) - params = { connection: read_options[:connection], query: update_query(id, stage) } + establish_connection(:read) - Utils::Postgres::Request.execute(params) + @read_connection.query(update_query(id, stage)) end def update_query(id, stage) diff --git a/lib/bas/utils/postgres/connection.rb b/lib/bas/utils/postgres/connection.rb new file mode 100644 index 0000000..7bfd9a1 --- /dev/null +++ b/lib/bas/utils/postgres/connection.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require "pg" + +module Utils + module Postgres + # This module is a PostgresDB utility to manage the connection to the database. + # + class Connection + def initialize(params) + @connection = PG::Connection.new(params[:connection]) + end + + def query(query) + results = if query.is_a? String + @connection.exec(query) + else + sentence, params = query + + @connection.exec_params(sentence, params) + end + + results.map { |result| result.transform_keys(&:to_sym) } + end + + def finish + @connection&.finish + @connection = nil + end + end + end +end From c62f1a19f564089c8c810ed3c359b1d6e7bae3ea Mon Sep 17 00:00:00 2001 From: XLEED_DEV Date: Wed, 16 Jul 2025 16:12:32 -0500 Subject: [PATCH 2/7] RSpec tests updated for existing classes --- lib/bas/shared_storage/POSTGRES.md | 91 ++++++++++++ spec/bas/bot/base_spec.rb | 38 ++++- spec/bas/shared_storage/postgres_spec.rb | 179 +++++++++++++++++++---- 3 files changed, 281 insertions(+), 27 deletions(-) create mode 100644 lib/bas/shared_storage/POSTGRES.md diff --git a/lib/bas/shared_storage/POSTGRES.md b/lib/bas/shared_storage/POSTGRES.md new file mode 100644 index 0000000..00525a1 --- /dev/null +++ b/lib/bas/shared_storage/POSTGRES.md @@ -0,0 +1,91 @@ +# SharedStorage::Postgres + +## Sequence Diagram + +```mermaid +sequenceDiagram + participant User as User/Code + participant SS as SharedStorage::Postgres + participant Req as Utils::Postgres::Request + participant PG as Postgres Server + + User->>SS: new(read_options, write_options) + activate SS + Note over SS: Instance created with options + + User->>SS: read() + activate SS + SS->>Req: execute({connection: read_options[:connection], query: read_query}) + activate Req + Req->>PG: PG::Connection.new(connection params) + activate PG + Note over Req,PG: Connection established + Req->>PG: exec_params(sentence, params) or exec(query) + PG->>Req: Query results + Req->>Req: map results to symbols + deactivate PG + Note over Req,PG: Connection not explicitly closed (auto-closed on GC) + Req->>SS: Return mapped results + deactivate Req + SS->>SS: Build Read response + SS->>User: Return Read object + deactivate SS + + User->>SS: set_in_process() + activate SS + alt avoid_process == true or id nil + SS->>User: Return nil + else + SS->>Req: execute({connection: read_options[:connection], query: update_query(id, "in process")}) + activate Req + Req->>PG: PG::Connection.new(connection params) + activate PG + Note over Req,PG: Connection established + Req->>PG: exec_params(sentence, params) + PG->>Req: Update result + deactivate PG + Note over Req,PG: Connection not explicitly closed + Req->>SS: Return result + deactivate Req + SS->>User: Return result + end + deactivate SS + + User->>SS: write(data) + activate SS + SS->>Req: execute({connection: write_options[:connection], query: write_query(data)}) + activate Req + Req->>PG: PG::Connection.new(connection params) + activate PG + Note over Req,PG: Connection established + Req->>PG: exec_params(sentence, params) + PG->>Req: Insert result + deactivate PG + Note over Req,PG: Connection not explicitly closed + Req->>SS: Return result + deactivate Req + SS->>User: Return result + deactivate SS + + User->>SS: set_processed() + activate SS + alt avoid_process == true or id nil + SS->>User: Return nil + else + SS->>Req: execute({connection: read_options[:connection], query: update_query(id, "processed")}) + activate Req + Req->>PG: PG::Connection.new(connection params) + activate PG + Note over Req,PG: Connection established + Req->>PG: exec_params(sentence, params) + PG->>Req: Update result + deactivate PG + Note over Req,PG: Connection not explicitly closed + Req->>SS: Return result + deactivate Req + SS->>User: Return result + end + deactivate SS + + deactivate SS +``` \ No newline at end of file diff --git a/spec/bas/bot/base_spec.rb b/spec/bas/bot/base_spec.rb index e3c60ad..d89f323 100644 --- a/spec/bas/bot/base_spec.rb +++ b/spec/bas/bot/base_spec.rb @@ -6,8 +6,9 @@ RSpec.describe Bas::Bot::Base do before do @options = {} - @shared_storage_reader = double(:shared_storage_reader, set_in_process: "in-process", set_processed: "processed") - @shared_storage_writer = double(:shared_storage_writer) + @shared_storage_reader = double(:shared_storage_reader, set_in_process: "in-process", set_processed: "processed", + close_connections: true) + @shared_storage_writer = double(:shared_storage_writer, close_connections: true) @bot = described_class.new(@options, @shared_storage_reader, @shared_storage_writer) end @@ -72,6 +73,39 @@ expect(@bot.process_response).to eql({}) expect(@bot.write_response).to eql({}) end + + it "closes the connections when close_connections_after_process is true" do + allow(@shared_storage_reader).to receive(:read).and_return(read_response) + allow(@shared_storage_writer).to receive(:write).and_return({}) + allow(@shared_storage_reader).to receive(:close_connections).and_return(true) + allow(@shared_storage_writer).to receive(:close_connections).and_return(true) + allow_any_instance_of(described_class).to receive(:process).and_return({ success: "ok" }) + allow(@shared_storage_reader).to receive(:respond_to?).with(:close_connections).and_return(true) + allow(@shared_storage_writer).to receive(:respond_to?).with(:close_connections).and_return(true) + + bot = described_class.new({ close_connections_after_process: true }, @shared_storage_reader, + @shared_storage_writer) + bot.execute + + expect(@shared_storage_reader).to have_received(:close_connections) + expect(@shared_storage_writer).to have_received(:close_connections) + end + + it "does not close the connections when close_connections_after_process is false" do + allow(@shared_storage_reader).to receive(:read).and_return(read_response) + allow(@shared_storage_writer).to receive(:write).and_return({}) + allow(@shared_storage_reader).to receive(:close_connections).and_return(true) + allow(@shared_storage_writer).to receive(:close_connections).and_return(true) + allow_any_instance_of(described_class).to receive(:process).and_return({ success: "ok" }) + + options = { close_connections_after_process: false } + bot = described_class.new(options, @shared_storage_reader, @shared_storage_writer) + + bot.execute + + expect(@shared_storage_reader).not_to have_received(:close_connections) + expect(@shared_storage_writer).not_to have_received(:close_connections) + end end describe ".unprocessable_response" do diff --git a/spec/bas/shared_storage/postgres_spec.rb b/spec/bas/shared_storage/postgres_spec.rb index 9ff74ee..727e823 100644 --- a/spec/bas/shared_storage/postgres_spec.rb +++ b/spec/bas/shared_storage/postgres_spec.rb @@ -4,26 +4,24 @@ require "bas/shared_storage/types/read" RSpec.describe Bas::SharedStorage::Postgres do - let(:connection) { { host: "localhost", port: 5432, dbname: "bas", user: "postgres", password: "postgres" } } - let(:read_options) { { connection:, db_table: "bas" } } - let(:write_options) { { connection:, db_table: "bas" } } + let(:connection_params) { { host: "localhost", port: 5432, dbname: "bas", user: "postgres", password: "postgres" } } + let(:read_options) { { connection: connection_params, db_table: "bas", tag: "test-tag" } } + let(:write_options) { { connection: connection_params, db_table: "bas", tag: "test-tag" } } let(:read_response) { Bas::SharedStorage::Types::Read.new } let(:process_success_response) { { success: "ok" } } let(:process_error_response) { { error: "there was an error" } } + let(:pg_connection) { instance_double(Utils::Postgres::Connection) } + let(:query_result) { [{ id: 1, data: '{ "success": "ok" }', inserted_at: "2024-11-12T00:00:00" }] } + before do - @pg_conn = instance_double(PG::Connection) - pg_result = instance_double(PG::Result) - - allow(PG::Connection).to receive(:new).and_return(@pg_conn) - allow(@pg_conn).to receive(:exec_params).and_return(pg_result) - allow(@pg_conn).to receive(:exec).and_return(pg_result) - allow(pg_result).to receive(:map).and_return([{ id: 1, data: "{ \"success\": \"ok\" }", - inserted_at: "2024-11-12T00:00:00" }]) + allow(Utils::Postgres::Connection).to receive(:new).and_return(pg_connection) + allow(pg_connection).to receive(:query).and_return(query_result) + allow(pg_connection).to receive(:finish) end describe ".read" do - it "search using the default where and params" do + it "searches using the default where and params" do shared_storage = described_class.new(read_options:, write_options:) expect(shared_storage.read).to be_a(Bas::SharedStorage::Types::Read) @@ -33,7 +31,7 @@ expect(shared_storage.read_response.inserted_at).to eql("2024-11-12T00:00:00") end - it "search using the configured where and params" do + it "searches using the configured where and params" do options = read_options.merge({ where: "id=$1", params: [2] }) shared_storage = described_class.new(read_options: options, write_options:) @@ -43,55 +41,186 @@ expect(shared_storage.read_response.data).to eql({ "success" => "ok" }) expect(shared_storage.read_response.inserted_at).to eql("2024-11-12T00:00:00") end + + it "reuses the read connection for multiple reads" do + shared_storage = described_class.new(read_options:, write_options:) + + expect(Utils::Postgres::Connection).to receive(:new).once.and_return(pg_connection) + + shared_storage.read + shared_storage.read # Second call should reuse connection + end + + it "handles empty query results" do + allow(pg_connection).to receive(:query).and_return([]) + shared_storage = described_class.new(read_options:, write_options:) + + expect(shared_storage.read).to be_a(Bas::SharedStorage::Types::Read) + expect(shared_storage.read_response.id).to be_nil + expect(shared_storage.read_response.data).to eql({}) + expect(shared_storage.read_response.inserted_at).to be_nil + end end describe ".write" do before { @shared_storage = described_class.new(read_options:, write_options:) } - it "save a success result" do + it "saves a success result" do @shared_storage.write(process_success_response) - expect(@shared_storage.write_response).not_to be(nil) + expect(@shared_storage.write_response).not_to be_nil end - it "save an error result" do + it "saves an error result" do @shared_storage.write(process_error_response) - expect(@shared_storage.write_response).not_to be(nil) + expect(@shared_storage.write_response).not_to be_nil + end + + it "reuses the write connection for multiple writes" do + expect(Utils::Postgres::Connection).to receive(:new).once.and_return(pg_connection) + + @shared_storage.write(process_success_response) + @shared_storage.write(process_error_response) # Second call should reuse connection + end + + it "uses separate connections for read and write when connection params differ" do + different_write_options = { connection: connection_params.merge(dbname: "different_db"), db_table: "bas", + tag: "test-tag" } + shared_storage = described_class.new(read_options:, write_options: different_write_options) + + expect(Utils::Postgres::Connection).to receive(:new).with(read_options[:connection]).once + expect(Utils::Postgres::Connection).to receive(:new).with(different_write_options[:connection]).once + + shared_storage.read + shared_storage.write(process_success_response) + end + end + + describe ".close_connections" do + it "closes both read and write connections" do + shared_storage = described_class.new(read_options:, write_options:) + + # Establish connections + shared_storage.read + shared_storage.write(process_success_response) + + expect(pg_connection).to receive(:finish).twice + + shared_storage.close_connections + end + + it "handles closing connections when none are established" do + shared_storage = described_class.new(read_options:, write_options:) + + expect { shared_storage.close_connections }.not_to raise_error + end + + it "allows re-establishing connections after closing" do + shared_storage = described_class.new(read_options:, write_options:) + + # First connection + shared_storage.read + shared_storage.close_connections + + # Should create new connection + expect(Utils::Postgres::Connection).to receive(:new).and_return(pg_connection) + shared_storage.read end end describe ".set_in_process" do - it "ignore execution if avoid_process is set to true" do + it "ignores execution if avoid_process is set to true" do options = read_options.merge({ avoid_process: true }) shared_storage = described_class.new(read_options: options, write_options:) - expect(shared_storage.set_in_process).to eql(nil) + expect(shared_storage.set_in_process).to be_nil end - it "update the record status to 'in process'" do + it "ignores execution if read_response.id is nil" do + allow(pg_connection).to receive(:query).and_return([]) shared_storage = described_class.new(read_options:, write_options:) + shared_storage.read + + expect(shared_storage.set_in_process).to be_nil + end + it "updates the record stage to 'in process'" do + shared_storage = described_class.new(read_options:, write_options:) shared_storage.read - expect(shared_storage.set_in_process).not_to be(nil) + expect(pg_connection).to receive(:query).with(["UPDATE bas SET stage=$1 WHERE id=$2", ["in process", 1]]) + shared_storage.set_in_process + end + + it "reuses the read connection for updates" do + shared_storage = described_class.new(read_options:, write_options:) + + # Should only create one connection (for read, then reused for update) + expect(Utils::Postgres::Connection).to receive(:new).once.and_return(pg_connection) + + shared_storage.read + shared_storage.set_in_process end end describe ".set_processed" do - it "ignore execution if avoid_process is set to true" do + it "ignores execution if avoid_process is set to true" do options = read_options.merge({ avoid_process: true }) shared_storage = described_class.new(read_options: options, write_options:) - expect(shared_storage.set_processed).to eql(nil) + expect(shared_storage.set_processed).to be_nil end - it "update the record status to 'processed'" do + it "ignores execution if read_response.id is nil" do + allow(pg_connection).to receive(:query).and_return([]) shared_storage = described_class.new(read_options:, write_options:) + shared_storage.read + expect(shared_storage.set_processed).to be_nil + end + + it "updates the record stage to 'processed'" do + shared_storage = described_class.new(read_options:, write_options:) shared_storage.read - expect(shared_storage.set_processed).not_to be(nil) + expect(pg_connection).to receive(:query).with(["UPDATE bas SET stage=$1 WHERE id=$2", ["processed", 1]]) + shared_storage.set_processed + end + + it "reuses the read connection for updates" do + shared_storage = described_class.new(read_options:, write_options:) + + # Should only create one connection (for read, then reused for update) + expect(Utils::Postgres::Connection).to receive(:new).once.and_return(pg_connection) + + shared_storage.read + shared_storage.set_processed + end + end + + describe "connection optimization" do + it "creates separate connections for read and write operations" do + shared_storage = described_class.new(read_options:, write_options:) + + # Should create two separate connections + expect(Utils::Postgres::Connection).to receive(:new).with(connection_params).twice.and_return(pg_connection) + + shared_storage.read + shared_storage.write(process_success_response) + end + + it "reuses connections across multiple operations" do + shared_storage = described_class.new(read_options:, write_options:) + + # Should only create connections once + expect(Utils::Postgres::Connection).to receive(:new).twice.and_return(pg_connection) + + shared_storage.read + shared_storage.set_in_process + shared_storage.write(process_success_response) + shared_storage.read + shared_storage.set_processed end end end From fd96a732ce848906750b57f86fecc52c1c43b47a Mon Sep 17 00:00:00 2001 From: Juan Pablo BC Date: Wed, 16 Jul 2025 20:54:57 -0500 Subject: [PATCH 3/7] Postgres connection class RSpec tests created --- spec/bas/utils/postgres/connection_spec.rb | 298 +++++++++++++++++++++ 1 file changed, 298 insertions(+) create mode 100644 spec/bas/utils/postgres/connection_spec.rb diff --git a/spec/bas/utils/postgres/connection_spec.rb b/spec/bas/utils/postgres/connection_spec.rb new file mode 100644 index 0000000..92ce360 --- /dev/null +++ b/spec/bas/utils/postgres/connection_spec.rb @@ -0,0 +1,298 @@ +# frozen_string_literal: true + +require "bas/utils/postgres/connection" + +RSpec.describe Utils::Postgres::Connection do + let(:connection_params) do + { + host: "localhost", + port: 5432, + dbname: "test_db", + user: "test_user", + password: "test_password" + } + end + + let(:pg_connection) { instance_double(PG::Connection) } + let(:pg_result) { instance_double(PG::Result) } + + before do + allow(PG::Connection).to receive(:new).and_return(pg_connection) + allow(pg_connection).to receive(:exec).and_return(pg_result) + allow(pg_connection).to receive(:exec_params).and_return(pg_result) + allow(pg_connection).to receive(:finish) + + # Mock the PG::Result to actually transform keys to symbols when map is called + allow(pg_result).to receive(:map) do + # Simulate the actual behavior where each result hash gets its keys transformed to symbols + raw_data = [ + { "id" => "1", "name" => "John Doe", "email" => "john@example.com" }, + { "id" => "2", "name" => "Jane Smith", "email" => "jane@example.com" } + ] + raw_data.map { |result| result.transform_keys(&:to_sym) } + end + end + + describe ".new" do + it "creates a new connection with the provided parameters" do + expect(PG::Connection).to receive(:new).with(connection_params).and_return(pg_connection) + + described_class.new({ connection: connection_params }) + end + + it "raises an error when connection parameters are invalid" do + allow(PG::Connection).to receive(:new).and_raise(PG::ConnectionBad, "connection failed") + + expect do + described_class.new({ connection: connection_params }) + end.to raise_error(PG::ConnectionBad, "connection failed") + end + end + + describe "#query" do + let(:connection) { described_class.new({ connection: connection_params }) } + + context "with a string query" do + let(:query_string) { "SELECT * FROM users WHERE active = true" } + let(:raw_results) do + [ + { "id" => "1", "name" => "John Doe", "email" => "john@example.com" }, + { "id" => "2", "name" => "Jane Smith", "email" => "jane@example.com" } + ] + end + + before do + allow(pg_result).to receive(:map).and_return(raw_results.map { |result| result.transform_keys(&:to_sym) }) + end + + it "executes a string query using exec" do + expect(pg_connection).to receive(:exec).with(query_string).and_return(pg_result) + + result = connection.query(query_string) + + expect(result).to eq([ + { id: "1", name: "John Doe", email: "john@example.com" }, + { id: "2", name: "Jane Smith", email: "jane@example.com" } + ]) + end + + it "transforms result keys to symbols" do + result = connection.query(query_string) + + expect(result.first.keys).to all(be_a(Symbol)) + expect(result.first).to have_key(:id) + expect(result.first).to have_key(:name) + expect(result.first).to have_key(:email) + end + + it "handles empty results" do + allow(pg_result).to receive(:map).and_return([]) + + result = connection.query(query_string) + + expect(result).to eq([]) + end + end + + context "with a parameterized query" do + let(:query_array) { ["SELECT * FROM users WHERE id = $1 AND active = $2", [1, true]] } + let(:raw_results) do + [ + { "id" => "1", "name" => "John Doe", "email" => "john@example.com" } + ] + end + + before do + allow(pg_result).to receive(:map).and_return(raw_results.map { |result| result.transform_keys(&:to_sym) }) + end + + it "executes a parameterized query using exec_params" do + sentence, params = query_array + expect(pg_connection).to receive(:exec_params).with(sentence, params).and_return(pg_result) + + result = connection.query(query_array) + + expect(result).to eq([ + { id: "1", name: "John Doe", email: "john@example.com" } + ]) + end + + it "transforms result keys to symbols for parameterized queries" do + result = connection.query(query_array) + + expect(result.first.keys).to all(be_a(Symbol)) + expect(result.first).to have_key(:id) + expect(result.first).to have_key(:name) + expect(result.first).to have_key(:email) + end + + it "handles parameterized queries with multiple parameters" do + complex_query = ["SELECT * FROM users WHERE age > $1 AND city = $2 AND active = $3", [18, "New York", true]] + allow(pg_connection).to receive(:exec_params).and_return(pg_result) + + connection.query(complex_query) + + expect(pg_connection).to have_received(:exec_params).with(complex_query[0], complex_query[1]) + end + end + + context "error handling" do + it "raises an error when exec fails" do + allow(pg_connection).to receive(:exec).and_raise(PG::Error, "syntax error") + + expect do + connection.query("INVALID SQL") + end.to raise_error(PG::Error, "syntax error") + end + + it "raises an error when exec_params fails" do + allow(pg_connection).to receive(:exec_params).and_raise(PG::Error, "parameter error") + + expect do + connection.query(["SELECT * FROM users WHERE id = $1", ["invalid"]]) + end.to raise_error(PG::Error, "parameter error") + end + + it "raises an error when query parameter is neither string nor array" do + allow(pg_result).to receive(:map).and_raise(NoMethodError, "undefined method") + + expect do + connection.query(123) + end.to raise_error(NoMethodError) + end + + # NOTE: The implementation doesn't validate array structure, so malformed arrays + # don't raise errors - they just behave unexpectedly + end + end + + describe "#finish" do + let(:connection) { described_class.new({ connection: connection_params }) } + + it "closes the underlying PG connection" do + expect(pg_connection).to receive(:finish) + + connection.finish + end + + it "sets the connection to nil after closing" do + connection.finish + + # We can't directly test the instance variable, but we can test that + # subsequent calls to finish don't raise errors + expect { connection.finish }.not_to raise_error + end + + it "handles finish when connection is already closed" do + connection.finish + + # Should not raise an error when called again + expect { connection.finish }.not_to raise_error + end + + it "handles finish when connection is nil" do + # Simulate a connection that was never established + allow(PG::Connection).to receive(:new).and_return(nil) + + connection = described_class.new({ connection: connection_params }) + expect { connection.finish }.not_to raise_error + end + end + + describe "integration scenarios" do + let(:connection) { described_class.new({ connection: connection_params }) } + + it "can execute multiple queries on the same connection" do + allow(pg_connection).to receive(:exec).and_return(pg_result) + allow(pg_result).to receive(:map).and_return([{ count: "5" }]) + + # First query + result1 = connection.query("SELECT COUNT(*) FROM users") + expect(result1).to eq([{ count: "5" }]) + + # Second query + result2 = connection.query("SELECT COUNT(*) FROM posts") + expect(result2).to eq([{ count: "5" }]) + + # Verify both queries were executed + expect(pg_connection).to have_received(:exec).with("SELECT COUNT(*) FROM users") + expect(pg_connection).to have_received(:exec).with("SELECT COUNT(*) FROM posts") + end + + it "can execute mixed string and parameterized queries" do + allow(pg_connection).to receive(:exec).and_return(pg_result) + allow(pg_connection).to receive(:exec_params).and_return(pg_result) + allow(pg_result).to receive(:map).and_return([{ result: "success" }]) + + # String query + connection.query("SELECT * FROM users") + + # Parameterized query + connection.query(["SELECT * FROM users WHERE id = $1", [1]]) + + expect(pg_connection).to have_received(:exec).with("SELECT * FROM users") + expect(pg_connection).to have_received(:exec_params).with("SELECT * FROM users WHERE id = $1", [1]) + end + + it "properly closes connection after use" do + allow(pg_connection).to receive(:exec).and_return(pg_result) + allow(pg_result).to receive(:map).and_return([]) + + connection.query("SELECT * FROM users") + connection.finish + + expect(pg_connection).to have_received(:finish) + end + end + + describe "edge cases" do + let(:connection) { described_class.new({ connection: connection_params }) } + + it "handles queries with special characters" do + special_query = "SELECT * FROM users WHERE name LIKE '%test%' AND email ~ '^[a-z]+@[a-z]+\\.com$'" + allow(pg_connection).to receive(:exec).and_return(pg_result) + allow(pg_result).to receive(:map).and_return([]) + + connection.query(special_query) + + expect(pg_connection).to have_received(:exec).with(special_query) + end + + it "handles parameterized queries with null values" do + null_query = ["SELECT * FROM users WHERE name = $1 AND email = $2", ["John", nil]] + allow(pg_connection).to receive(:exec_params).and_return(pg_result) + allow(pg_result).to receive(:map).and_return([]) + + connection.query(null_query) + + expect(pg_connection).to have_received(:exec_params).with(null_query[0], null_query[1]) + end + + it "handles queries with empty string parameters" do + empty_query = ["SELECT * FROM users WHERE name = $1", [""]] + allow(pg_connection).to receive(:exec_params).and_return(pg_result) + allow(pg_result).to receive(:map).and_return([]) + + connection.query(empty_query) + + expect(pg_connection).to have_received(:exec_params).with(empty_query[0], empty_query[1]) + end + + it "handles results with complex data types" do + complex_results = [ + { "id" => "1", "data" => "{\"key\": \"value\"}", "array" => "{1,2,3}", "timestamp" => "2024-01-01 12:00:00" } + ] + allow(pg_connection).to receive(:exec).and_return(pg_result) + allow(pg_result).to receive(:map).and_return(complex_results.map { |result| result.transform_keys(&:to_sym) }) + + result = connection.query("SELECT * FROM complex_table") + + expect(result.first).to eq({ + id: "1", + data: "{\"key\": \"value\"}", + array: "{1,2,3}", + timestamp: "2024-01-01 12:00:00" + }) + end + end +end From 67a65fba04b5b1c7fc5832f7b1c0dc66fae509ee Mon Sep 17 00:00:00 2001 From: Juan Pablo BC Date: Wed, 16 Jul 2025 20:55:39 -0500 Subject: [PATCH 4/7] Docstrings added to postgres connection class --- lib/bas/utils/postgres/connection.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/bas/utils/postgres/connection.rb b/lib/bas/utils/postgres/connection.rb index 7bfd9a1..c84efb8 100644 --- a/lib/bas/utils/postgres/connection.rb +++ b/lib/bas/utils/postgres/connection.rb @@ -4,11 +4,14 @@ module Utils module Postgres - # This module is a PostgresDB utility to manage the connection to the database. + # This module is a PostgresDB utility to establish connections to a Postgres database + # and execute raw or parameterized queries. # class Connection def initialize(params) + puts "Instantiating connection with params: #{params}" @connection = PG::Connection.new(params[:connection]) + puts "Conntection instantiated" end def query(query) From bca7033707136ab129b2e1d9d15b8093fab6c1a3 Mon Sep 17 00:00:00 2001 From: Juan Pablo BC Date: Thu, 17 Jul 2025 16:44:44 -0500 Subject: [PATCH 5/7] Fix issue on postgres connection params --- lib/bas/utils/postgres/connection.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/bas/utils/postgres/connection.rb b/lib/bas/utils/postgres/connection.rb index c84efb8..488bdb1 100644 --- a/lib/bas/utils/postgres/connection.rb +++ b/lib/bas/utils/postgres/connection.rb @@ -9,9 +9,7 @@ module Postgres # class Connection def initialize(params) - puts "Instantiating connection with params: #{params}" - @connection = PG::Connection.new(params[:connection]) - puts "Conntection instantiated" + @connection = PG::Connection.new(params) end def query(query) From a03d3db7a6d095ffe085a7c30d95cd87c8bd2025 Mon Sep 17 00:00:00 2001 From: Juan Pablo BC Date: Fri, 18 Jul 2025 14:56:36 -0500 Subject: [PATCH 6/7] Coderrabit suggestions applied. Tests updated. Linter fixes --- lib/bas/bot/base.rb | 3 +- lib/bas/shared_storage/POSTGRES.md | 91 ---------------------- lib/bas/shared_storage/base.rb | 5 +- lib/bas/utils/postgres/connection.rb | 11 ++- spec/bas/bot/base_spec.rb | 2 +- spec/bas/utils/postgres/connection_spec.rb | 23 +++++- 6 files changed, 33 insertions(+), 102 deletions(-) delete mode 100644 lib/bas/shared_storage/POSTGRES.md diff --git a/lib/bas/bot/base.rb b/lib/bas/bot/base.rb index 7a161fc..f675160 100644 --- a/lib/bas/bot/base.rb +++ b/lib/bas/bot/base.rb @@ -15,7 +15,8 @@ class Base attr_accessor :read_response, :process_response, :write_response def initialize(options, shared_storage_reader, shared_storage_writer = nil) - @process_options = options || { close_connections_after_process: true } + default_options = { close_connections_after_process: true } + @process_options = default_options.merge(options || {}) @shared_storage_reader = shared_storage_reader @shared_storage_writer = shared_storage_writer || shared_storage_reader end diff --git a/lib/bas/shared_storage/POSTGRES.md b/lib/bas/shared_storage/POSTGRES.md deleted file mode 100644 index 00525a1..0000000 --- a/lib/bas/shared_storage/POSTGRES.md +++ /dev/null @@ -1,91 +0,0 @@ -# SharedStorage::Postgres - -## Sequence Diagram - -```mermaid -sequenceDiagram - participant User as User/Code - participant SS as SharedStorage::Postgres - participant Req as Utils::Postgres::Request - participant PG as Postgres Server - - User->>SS: new(read_options, write_options) - activate SS - Note over SS: Instance created with options - - User->>SS: read() - activate SS - SS->>Req: execute({connection: read_options[:connection], query: read_query}) - activate Req - Req->>PG: PG::Connection.new(connection params) - activate PG - Note over Req,PG: Connection established - Req->>PG: exec_params(sentence, params) or exec(query) - PG->>Req: Query results - Req->>Req: map results to symbols - deactivate PG - Note over Req,PG: Connection not explicitly closed (auto-closed on GC) - Req->>SS: Return mapped results - deactivate Req - SS->>SS: Build Read response - SS->>User: Return Read object - deactivate SS - - User->>SS: set_in_process() - activate SS - alt avoid_process == true or id nil - SS->>User: Return nil - else - SS->>Req: execute({connection: read_options[:connection], query: update_query(id, "in process")}) - activate Req - Req->>PG: PG::Connection.new(connection params) - activate PG - Note over Req,PG: Connection established - Req->>PG: exec_params(sentence, params) - PG->>Req: Update result - deactivate PG - Note over Req,PG: Connection not explicitly closed - Req->>SS: Return result - deactivate Req - SS->>User: Return result - end - deactivate SS - - User->>SS: write(data) - activate SS - SS->>Req: execute({connection: write_options[:connection], query: write_query(data)}) - activate Req - Req->>PG: PG::Connection.new(connection params) - activate PG - Note over Req,PG: Connection established - Req->>PG: exec_params(sentence, params) - PG->>Req: Insert result - deactivate PG - Note over Req,PG: Connection not explicitly closed - Req->>SS: Return result - deactivate Req - SS->>User: Return result - deactivate SS - - User->>SS: set_processed() - activate SS - alt avoid_process == true or id nil - SS->>User: Return nil - else - SS->>Req: execute({connection: read_options[:connection], query: update_query(id, "processed")}) - activate Req - Req->>PG: PG::Connection.new(connection params) - activate PG - Note over Req,PG: Connection established - Req->>PG: exec_params(sentence, params) - PG->>Req: Update result - deactivate PG - Note over Req,PG: Connection not explicitly closed - Req->>SS: Return result - deactivate Req - SS->>User: Return result - end - deactivate SS - - deactivate SS -``` \ No newline at end of file diff --git a/lib/bas/shared_storage/base.rb b/lib/bas/shared_storage/base.rb index ddfe1d9..d15ab85 100644 --- a/lib/bas/shared_storage/base.rb +++ b/lib/bas/shared_storage/base.rb @@ -21,10 +21,7 @@ def set_in_process; end def set_processed; end - def close_connections - # TODO: Leave this method empty after testing - puts "Closing connection for #{self.class.name}" - end + def close_connections; end protected diff --git a/lib/bas/utils/postgres/connection.rb b/lib/bas/utils/postgres/connection.rb index 488bdb1..29c5cf4 100644 --- a/lib/bas/utils/postgres/connection.rb +++ b/lib/bas/utils/postgres/connection.rb @@ -16,8 +16,9 @@ def query(query) results = if query.is_a? String @connection.exec(query) else - sentence, params = query + validate_query(query) + sentence, params = query @connection.exec_params(sentence, params) end @@ -28,6 +29,14 @@ def finish @connection&.finish @connection = nil end + + private + + def validate_query(query) + return if query.is_a?(Array) && query.size == 2 && query[0].is_a?(String) && query[1].is_a?(Array) + + raise ArgumentError, "Parameterized query must be an array of [sentence (String), params (Array)]" + end end end end diff --git a/spec/bas/bot/base_spec.rb b/spec/bas/bot/base_spec.rb index d89f323..413ad02 100644 --- a/spec/bas/bot/base_spec.rb +++ b/spec/bas/bot/base_spec.rb @@ -5,7 +5,7 @@ RSpec.describe Bas::Bot::Base do before do - @options = {} + @options = { close_connections_after_process: true } @shared_storage_reader = double(:shared_storage_reader, set_in_process: "in-process", set_processed: "processed", close_connections: true) @shared_storage_writer = double(:shared_storage_writer, close_connections: true) diff --git a/spec/bas/utils/postgres/connection_spec.rb b/spec/bas/utils/postgres/connection_spec.rb index 92ce360..8dd7217 100644 --- a/spec/bas/utils/postgres/connection_spec.rb +++ b/spec/bas/utils/postgres/connection_spec.rb @@ -37,7 +37,7 @@ it "creates a new connection with the provided parameters" do expect(PG::Connection).to receive(:new).with(connection_params).and_return(pg_connection) - described_class.new({ connection: connection_params }) + described_class.new(connection_params) end it "raises an error when connection parameters are invalid" do @@ -158,11 +158,26 @@ expect do connection.query(123) - end.to raise_error(NoMethodError) + end.to raise_error(ArgumentError) end - # NOTE: The implementation doesn't validate array structure, so malformed arrays - # don't raise errors - they just behave unexpectedly + it "raises ArgumentError for parameterized query with wrong array size" do + expect do + connection.query(["SELECT * FROM users WHERE id = $1"]) + end.to raise_error(ArgumentError, "Parameterized query must be an array of [sentence (String), params (Array)]") + end + + it "raises ArgumentError for parameterized query with non-string sentence" do + expect do + connection.query([1, [1]]) + end.to raise_error(ArgumentError, "Parameterized query must be an array of [sentence (String), params (Array)]") + end + + it "raises ArgumentError for parameterized query with non-array params" do + expect do + connection.query(["SELECT * FROM users WHERE id = $1", 1]) + end.to raise_error(ArgumentError, "Parameterized query must be an array of [sentence (String), params (Array)]") + end end end From 8e0ec854dd2e4243613ecaf9dd7b04cab473f999 Mon Sep 17 00:00:00 2001 From: Juan Pablo BC Date: Mon, 21 Jul 2025 15:13:34 -0500 Subject: [PATCH 7/7] Unused mock removed from connection_spec --- spec/bas/utils/postgres/connection_spec.rb | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/spec/bas/utils/postgres/connection_spec.rb b/spec/bas/utils/postgres/connection_spec.rb index 8dd7217..60074ca 100644 --- a/spec/bas/utils/postgres/connection_spec.rb +++ b/spec/bas/utils/postgres/connection_spec.rb @@ -21,16 +21,6 @@ allow(pg_connection).to receive(:exec).and_return(pg_result) allow(pg_connection).to receive(:exec_params).and_return(pg_result) allow(pg_connection).to receive(:finish) - - # Mock the PG::Result to actually transform keys to symbols when map is called - allow(pg_result).to receive(:map) do - # Simulate the actual behavior where each result hash gets its keys transformed to symbols - raw_data = [ - { "id" => "1", "name" => "John Doe", "email" => "john@example.com" }, - { "id" => "2", "name" => "Jane Smith", "email" => "jane@example.com" } - ] - raw_data.map { |result| result.transform_keys(&:to_sym) } - end end describe ".new" do