Merge pull request #146 from rsim/improvement/agents_instructions

Add agents instructions, improve code style

Author: rsim

.github/copilot-instructions.md

@@ -1,11 +1 @@
-## Project Overview
-
-mondrian-olap is a JRuby gem for performing multidimensional queries of relational database data using Mondrian OLAP Java library.
-
-## Technology Stack
-
-- JRuby 9.4 or later (compatible with Ruby 3.1+)
-- Java 8 or later
-- Mondrian OLAP Java library from a fork https://github.com/rsim/mondrian/tree/9.3.0.0-rsim
-- Databases: PostgreSQL, MySQL, Oracle, Microsoft SQL Server, ClickHouse or other JDBC compatible databases
-- Testing: RSpec
+Use AGENTS.md for code style and guidelines.

.gitignore

@@ -3,7 +3,9 @@
 .DS_Store
 .autotest
 .ruby-*
-/mise.local.toml
+mise.local.toml
+CLAUDE.local.md
+.claude
 coverage
 doc
 pkg

AGENTS.md

@@ -0,0 +1,111 @@
+# mondrian-olap
+
+This file provides guidance to AI coding agents working with this repository.
+
+## Overview
+
+mondrian-olap is a JRuby gem for performing multidimensional queries of relational database data using Mondrian OLAP Java library.
+
+## Codebase Structure
+
+### Main Entry Points
+
+- `lib/mondrian-olap.rb` - Main gem entry point that requires all components
+- `lib/mondrian/olap.rb` - Core module and Java library initialization
+- `lib/mondrian/olap/connection.rb` - Database connection management
+- `lib/mondrian/olap/schema.rb` - OLAP schema definition DSL
+- `lib/mondrian/olap/cube.rb` - Cube operations and queries
+- `lib/mondrian/olap/query.rb` - Query builder for MDX-like queries
+- `lib/mondrian/olap/result.rb` - Query result processing
+
+### Key Modules
+
+- `Mondrian::OLAP::Connection` - Manages connections to OLAP data sources
+- `Mondrian::OLAP::Schema` - Provides DSL for defining OLAP schemas programmatically
+- `Mondrian::OLAP::Cube` - Represents OLAP cubes and provides query interface
+- `Mondrian::OLAP::Query` - Builds and executes MDX-like queries
+- `Mondrian::OLAP::Result` - Handles query results with axes and cells
+
+### Java Integration
+
+- `lib/mondrian/jars/` - Contains Mondrian OLAP Java library JAR files
+- The gem bridges Ruby code with Java Mondrian library using JRuby's Java integration
+- Java objects are wrapped in Ruby classes to provide idiomatic Ruby API
+
+## Common Workflows
+
+### Making Changes
+
+1. **Adding new schema features** - Modify `lib/mondrian/olap/schema.rb` and add corresponding specs in
+   `spec/schema_definition_spec.rb`
+2. **Extending query capabilities** - Update `lib/mondrian/olap/query.rb` and test in `spec/query_spec.rb`
+3. **Connection enhancements** - Change `lib/mondrian/olap/connection.rb` with tests in `spec/connection_spec.rb`
+4. **Cube operations** - Modify `lib/mondrian/olap/cube.rb` and add specs in `spec/cube_spec.rb`
+
+### Testing Changes
+
+1. Write or update RSpec tests for the changed functionality.
+2. Run specific test file: `rspec spec/cube_spec.rb`.
+3. Run all tests with default database: `rake spec`.
+4. Test with specific databases: `rake spec:postgresql`, `rake spec:mysql`, etc.
+5. Ensure tests pass with multiple database backends before finalizing changes.
+
+## Technology Stack
+
+- **JRuby** 9.4 or later (compatible with Ruby 3.1+)
+- **Java** 8 or later LTS version
+- **Mondrian OLAP** Java library from a fork https://github.com/rsim/mondrian/tree/9.3.0.0-rsim
+- **Databases**: PostgreSQL, MySQL, Oracle, Microsoft SQL Server, ClickHouse or other JDBC compatible databases
+- **Testing**: RSpec
+
+### JRuby-Specific Considerations
+
+- This gem requires JRuby and will not work with standard MRI Ruby.
+- Use JRuby's Java integration features to interact with Mondrian Java library.
+- Java objects can be accessed directly in JRuby code.
+- JDBC drivers are used for database connections instead of native Ruby database adapters.
+
+## Code Style and Guidelines
+
+### General
+
+- Use meaningful semantic names for variables, methods, and classes.
+- Use query and command method conventions.
+  - Query methods use nouns and do not modify state and do not have side effects. Boolean methods end with `?`.
+  - Command methods use verbs that describe the action being taken and may modify state.
+- Use consistent naming, use the same terminology throughout the codebase.
+- Do not use similar variable or method names for different data or objects.
+- Write comments to explain why something is done, not what is done.
+- Write comments only when it is not obvious from the code.
+- Start full sentence comments with a capital letter.
+- Prefer self-explanatory code with semantic names over detailed comments.
+- Keep methods small and focused on a single task.
+- Write simple readable code. Do not obfuscate simple logic.
+- Validate correct spelling for variable, method, class names, as well as for comments.
+
+### Ruby
+
+- Do not modify objects (like Hash and Array) that are referenced by argument variables.
+  This might cause unexpected side effects in the caller. It is OK to assign a new object to an argument variable.
+- Return collections (arrays or other) from methods with plural names. Do not return nil from methods with plural names,
+  return empty collections in such cases.
+- Use &:method for collections:
+  `collection.map(&:method)` instead of `collection.map { |item| item.method }`.
+- Use safe navigation operator `&.` when calling methods on objects that might be nil.
+- When continuing the method call on the next line, then end the first line with a dot.
+- Use the new hash syntax `key: 'value'` instead of the old syntax `:key => 'value'`.
+- Use the old hash syntax only for rake task dependencies, for example, `task :build => :compile`.
+- Use simple parentheses declaring an array of strings `%w()` instead of other symbols like `%w[]`.
+- Use frozen string literal comments for all Ruby files and ensure that frozen strings are not modified.
+
+### Testing
+
+- Use RSpec for Ruby testing.
+- Run individual RSpec test file with e.g. `rspec spec/cube_spec.rb`.
+- Run all RSpec tests with `rake spec` (with the default `mysql` database).
+- Run all tests with a specified database:
+  `rake spec:mysql`, `rake spec:postgresql`, `rake spec:sqlserver`, `rake spec:oracle`
+- In most cases use RSpec should syntax and not expect syntax, for example, `result.should == expected`.
+- Use RSpec expect syntax only for block expectations, for example, `expect { action }.to raise_error(SomeError)`.
+- Test data is located in `spec/support/data/` directory.
+- Database-specific schema fixtures are in `spec/fixtures/` directory.

CLAUDE.md

@@ -0,0 +1 @@
+@AGENTS.md

README.md

@@ -1,5 +1,4 @@
 [![GitHub Actions status](https://github.com/rsim/mondrian-olap/actions/workflows/tests.yml/badge.svg)](https://github.com/rsim/mondrian-olap/actions/workflows/tests.yml)
-[![AppVeyor status](https://ci.appveyor.com/api/projects/status/08xd4tyty2k3wxba/branch/master?svg=true)](https://ci.appveyor.com/project/rsim/mondrian-olap)
 
 mondrian-olap
 =============

appveyor.yml

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'java'
 require 'nokogiri'
 

lib/mondrian/olap.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module Mondrian
   module OLAP
     class Connection
@@ -19,7 +21,7 @@ def initialize(params = {})
 
       def connect
         Error.wrap_native_exception do
-          # hack to call private constructor of MondrianOlap4jDriver
+          # Hack to call private constructor of MondrianOlap4jDriver
           # to avoid using DriverManager which fails to load JDBC drivers
           # because of not seeing JRuby required jar files
           cons = Java::MondrianOlap4j::MondrianOlap4jDriver.java_class.declared_constructor
@@ -30,7 +32,7 @@ def connect
           props.setProperty('JdbcUser', @params[:username]) if @params[:username]
           props.setProperty('JdbcPassword', @params[:password]) if @params[:password]
 
-          # on Oracle increase default row prefetch size
+          # On Oracle increase default row prefetch size
           # as default 10 is very low and slows down loading of all dimension members
           if @driver == 'oracle'
             prefetch_rows = @params[:prefetch_rows] || 100
@@ -39,7 +41,7 @@ def connect
 
           conn_string = connection_string
 
-          # latest Mondrian version added ClassResolver which uses current thread class loader to load some classes
+          # Latest Mondrian version added ClassResolver which uses current thread class loader to load some classes
           # therefore need to set it to JRuby class loader to ensure that Mondrian classes are found
           # (e.g. when running mondrian-olap inside OSGi container)
           current_thread = Java::JavaLang::Thread.currentThread
@@ -53,7 +55,7 @@ def connect
 
           @raw_connection = @raw_jdbc_connection.unwrap(Java::OrgOlap4j::OlapConnection.java_class)
           @raw_catalog = @raw_connection.getOlapCatalog
-          # currently it is assumed that there is just one schema per connection catalog
+          # Currently it is assumed that there is just one schema per connection catalog
           @raw_schema = @raw_catalog.getSchemas.first
           @raw_mondrian_connection = @raw_connection.getMondrianConnection
           @raw_schema_reader = @raw_mondrian_connection.getSchemaReader
@@ -89,7 +91,7 @@ def execute(query_string, parameters = {})
         end
       end
 
-      # access mondrian.olap.Parameter object
+      # Access mondrian.olap.Parameter object
       def mondrian_parameter(parameter_name)
         Error.wrap_native_exception do
           @raw_schema_reader.getParameter(parameter_name)
@@ -144,7 +146,7 @@ def self.raw_schema_key(schema_key)
       end
 
       def cube_names
-        @raw_schema.getCubes.map{|c| c.getName}
+        @raw_schema.getCubes.map(&:getName)
       end
 
       def cube(name)
@@ -193,7 +195,7 @@ def role_name
       end
 
       def role_names
-        # workaround to access non-public method (was not public when using inside Torquebox)
+        # Workaround to access non-public method
         # @raw_connection.getRoleNames.to_a
         @raw_connection.java_method(:getRoleNames).call.to_a
       end
@@ -206,7 +208,7 @@ def role_name=(name)
 
       def role_names=(names)
         Error.wrap_native_exception do
-          # workaround to access non-public method (was not public when using inside Torquebox)
+          # Workaround to access non-public method
           # @raw_connection.setRoleNames(Array(names))
           names = Array(names)
           @raw_connection.java_method(:setRoleNames, [Java::JavaUtil::List.java_class]).call(names)
@@ -225,7 +227,7 @@ def locale=(locale)
         @raw_connection.setLocale(java_locale)
       end
 
-      # access MondrianServer instance
+      # Access MondrianServer instance
       def mondrian_server
         Error.wrap_native_exception do
           @raw_connection.getMondrianConnection.getServer
@@ -325,11 +327,11 @@ def jdbc_uri
 
       def connection_string
         string = "jdbc:mondrian:Jdbc=#{quote_string(jdbc_uri)};JdbcDrivers=#{jdbc_driver};"
-        # by default use content checksum to reload schema when catalog has changed
+        # By default use content checksum to reload schema when catalog has changed
         string += "UseContentChecksum=true;" unless @params[:use_content_checksum] == false
         string += "PinSchemaTimeout=#{@params[:pin_schema_timeout]};" if @params[:pin_schema_timeout]
         if role = @params[:role] || @params[:roles]
-          roles = Array(role).map{|r| r && r.to_s.gsub(',', ',,')}.compact
+          roles = Array(role).map { |r| r && r.to_s.gsub(',', ',,') }.compact
           string += "Role=#{quote_string(roles.join(','))};" unless roles.empty?
         end
         if locale = @params[:locale]
@@ -371,14 +373,14 @@ def jdbc_uri_mysql
       alias_method :jdbc_uri_mariadb, :jdbc_uri_generic
 
       def jdbc_uri_oracle
-        # connection using TNS alias
+        # Connection using TNS alias
         if @params[:database] && !@params[:host] && !@params[:url] && ENV['TNS_ADMIN']
           "jdbc:oracle:thin:@#{@params[:database]}"
         else
           @params[:url] || begin
             database = @params[:database]
             unless database =~ %r{^(:|/)}
-              # assume database is a SID if no colon or slash are supplied (backward-compatibility)
+              # Assume database is a SID if no colon or slash are supplied (backward-compatibility)
               database = ":#{database}"
             end
             "jdbc:oracle:thin:@#{@params[:host] || 'localhost'}:#{@params[:port] || 1521}#{database}"
@@ -470,7 +472,7 @@ def catalog_content
         if @params[:catalog_content]
           @params[:catalog_content]
         elsif @params[:schema]
-          @params[:schema].to_xml(:driver => @driver)
+          @params[:schema].to_xml(driver: @driver)
         else
           raise ArgumentError, "Specify catalog with :catalog, :catalog_content or :schema option"
         end
@@ -483,7 +485,7 @@ def quote_string(string)
       def set_statement_parameters(statement, parameters)
         if parameters && !parameters.empty?
           parameters = parameters.dup
-          # define addtional parameters which can be accessed from user defined functions
+          # Define additional parameters which can be accessed from user defined functions
           if define_parameters = parameters.delete(:define_parameters)
             query_validator = statement.getQuery.createValidator
             define_parameters.each do |dp_name, dp_value|

lib/mondrian/olap/connection.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'forwardable'
 
 module Mondrian
@@ -66,7 +68,7 @@ def virtual?
       end
 
       def dimensions
-        @dimenstions ||= @raw_cube.getDimensions.map { |d| dimension_from_raw(d) }
+        @dimensions ||= @raw_cube.getDimensions.map { |d| dimension_from_raw(d) }
       end
 
       def dimension_names
@@ -113,8 +115,7 @@ def member_by_segments(*segment_names)
         raw_member && Member.new(raw_member)
       end
 
-      def_delegators :@cache_control, :flush_region_cache_with_segments, :flush_region_cache_with_segments
-      def_delegators :@cache_control, :flush_region_cache_with_full_names, :flush_region_cache_with_full_names
+      def_delegators :@cache_control, :flush_region_cache_with_segments, :flush_region_cache_with_full_names
 
       private
 
@@ -187,7 +188,6 @@ def annotations
       def visible?
         @raw_dimension.isVisible
       end
-
     end
 
     class Hierarchy
@@ -274,7 +274,6 @@ def annotations
       def visible?
         @raw_hierarchy.isVisible
       end
-
     end
 
     class Level
@@ -372,7 +371,6 @@ def annotations
       def visible?
         @raw_level.isVisible
       end
-
     end
 
     class Member
@@ -461,9 +459,7 @@ def descendants_at_level(level)
 
           members = [self]
           (descendants_level_index - current_level_index).times do
-            members = members.map do |member|
-              member.children
-            end.flatten
+            members = members.flat_map(&:children)
           end
           members
         end
@@ -532,7 +528,7 @@ def flush_region_cache_with_segments(*segment_names)
       end
 
       def flush_region_cache_with_full_names(*full_names)
-        members = full_names.map { |name| @cube.member(*name).mondrian_member }
+        members = full_names.map { |name| @cube.member(name).mondrian_member }
         flush(members)
       end