#122: Apply OO style (#123)

#122: Apply OO rules according to style guide.

Co-authored-by: chπ <christoph.pirkl@exasol.com>

dependencies.md

@@ -34,10 +34,10 @@
 | [Exec Maven Plugin][42] | [Apache License 2][27] |
 | [OpenFastTrace Maven Plugin][44] | [GNU General Public License v3.0][45] |
 | [Build Helper Maven Plugin][46] | [The MIT License][47] |
-| [Apache Maven JAR Plugin][48]   | [Apache License, Version 2.0][23] |
-| [Maven PlantUML plugin][50]  | [Apache License
-  Version 2.0, January 2004][51] |
-| [error-code-crawler-maven-plugin][52]  | [MIT][3]  |
+| [error-code-crawler-maven-plugin][48] | [MIT][3]   |
+| [Apache Maven JAR Plugin][50] | [Apache License, Version 2.0][23] |
+| [Maven PlantUML plugin][52]  | [Apache License
+  Version 2.0, January 2004][53] |
 | [Maven Clean Plugin][54] | [The Apache Software License, Version 2.0][27] |
 | [Maven Resources Plugin][56] | [The Apache Software License, Version 2.0][27] |
 | [Maven Install Plugin][58] | [The Apache Software License, Version 2.0][27] |
@@ -65,7 +65,7 @@
 [21]: http://www.gnu.org/licenses/lgpl.txt
 [2]: https://github.com/exasol/exasol-testcontainers
 [38]: https://www.jacoco.org/jacoco/trunk/doc/maven.html
-[51]: https://www.apache.org/licenses/LICENSE-2.0
+[53]: https://www.apache.org/licenses/LICENSE-2.0
 [8]: https://github.com/exasol/hamcrest-resultset-matcher
 [30]: http://zlika.github.io/reproducible-build-maven-plugin
 [41]: https://github.com/exasol/project-keeper/blob/main/LICENSE
@@ -82,11 +82,11 @@
 [28]: https://sonatype.github.io/ossindex-maven/maven-plugin/
 [4]: https://testcontainers.org
 [46]: http://www.mojohaus.org/build-helper-maven-plugin/
-[50]: https://github.com/Huluvu424242/plantuml-maven-plugin
+[52]: https://github.com/Huluvu424242/plantuml-maven-plugin
 [6]: http://hamcrest.org/JavaHamcrest/
 [14]: http://www.slf4j.org
 [60]: http://maven.apache.org/plugins/maven-deploy-plugin/
 [62]: http://maven.apache.org/plugins/maven-site-plugin/
 [56]: http://maven.apache.org/plugins/maven-resources-plugin/
-[52]: https://github.com/exasol/error-code-crawler-maven-plugin
-[48]: https://maven.apache.org/plugins/maven-jar-plugin/
+[48]: https://github.com/exasol/error-code-crawler-maven-plugin
+[50]: https://maven.apache.org/plugins/maven-jar-plugin/

doc/changes/changes_1.1.0.md

@@ -28,27 +28,31 @@ In this release we have implemented GROUP BY capabilities support. The release a
 
 ### Test Dependency Updates
 
-* Updated `com.exasol:exasol-jdbc:7.1.2` to `7.1.7`
+* Updated `com.exasol:exasol-jdbc:7.1.2` to `7.1.10`
 * Updated `com.exasol:exasol-testcontainers:5.1.1` to `6.1.1`
 * Added `com.exasol:maven-project-version-getter:1.1.0`
-* Updated `com.exasol:test-db-builder-java:3.2.1` to `3.3.1`
+* Updated `com.exasol:test-db-builder-java:3.2.1` to `3.3.2`
 * Updated `org.junit.jupiter:junit-jupiter-engine:5.8.1` to `5.8.2`
 * Updated `org.junit.jupiter:junit-jupiter-params:5.8.1` to `5.8.2`
 * Removed `org.junit.platform:junit-platform-runner:1.8.1`
 * Updated `org.slf4j:slf4j-jdk14:1.7.32` to `1.7.36`
-* Updated `org.testcontainers:junit-jupiter:1.16.2` to `1.17.1`
+* Updated `org.testcontainers:junit-jupiter:1.16.2` to `1.17.2`
 
 ### Plugin Dependency Updates
 
-* Updated `com.exasol:project-keeper-maven-plugin:1.3.2` to `2.3.0`
+* Updated `com.exasol:error-code-crawler-maven-plugin:0.1.1` to `1.1.1`
+* Updated `com.exasol:project-keeper-maven-plugin:1.3.2` to `2.4.6`
 * Added `com.github.funthomas424242:plantuml-maven-plugin:1.5.2`
 * Updated `io.github.zlika:reproducible-build-maven-plugin:0.13` to `0.15`
-* Updated `org.apache.maven.plugins:maven-compiler-plugin:3.8.1` to `3.9.0`
+* Updated `org.apache.maven.plugins:maven-compiler-plugin:3.8.1` to `3.10.1`
 * Updated `org.apache.maven.plugins:maven-enforcer-plugin:3.0.0-M3` to `3.0.0`
 * Updated `org.apache.maven.plugins:maven-failsafe-plugin:3.0.0-M4` to `3.0.0-M5`
+* Updated `org.apache.maven.plugins:maven-jar-plugin:3.2.0` to `3.2.2`
 * Updated `org.apache.maven.plugins:maven-surefire-plugin:3.0.0-M4` to `3.0.0-M5`
+* Updated `org.codehaus.mojo:build-helper-maven-plugin:3.2.0` to `3.3.0`
 * Added `org.codehaus.mojo:flatten-maven-plugin:1.2.7`
-* Updated `org.codehaus.mojo:versions-maven-plugin:2.7` to `2.8.1`
+* Updated `org.codehaus.mojo:versions-maven-plugin:2.7` to `2.10.0`
 * Added `org.itsallcode:openfasttrace-maven-plugin:1.5.0`
-* Updated `org.jacoco:jacoco-maven-plugin:0.8.5` to `0.8.7`
+* Updated `org.jacoco:jacoco-maven-plugin:0.8.5` to `0.8.8`
 * Added `org.sonarsource.scanner.maven:sonar-maven-plugin:3.9.1.2184`
+* Updated `org.sonatype.ossindex.maven:ossindex-maven-plugin:3.1.0` to `3.2.0`

doc/changes/changes_1.2.0.md

@@ -1,10 +1,10 @@
-# Exasol Row Level Security (Lua) 1.2.0, released 2022-05-??
+# Exasol Row Level Security (Lua) 1.2.0, released 2022-06-08
 
-Code name: `ORDER BY`, `LIMIT`, more functions and unified error handling
+Code name: LIMIT, GROUP BY
 
 ## Summary
 
-Release 1.2.0 of `row-level-security-lua` upgrades the base module to `virtual-schema-common-lua` 1.1.0, which brings support for the `ORDER BY` and `LIMIT` clause as well as more scalar function coverage and uniform error reporting. 
+RLS (Lua) version 1.2.0 brings support for `LIMIT` with an optional offset and for `GROUP BY` clauses.
 
 Note that this version requires Exasol 7.1.10 or later to run stable, because earlier versions had a problem in handling Lua's `pcall` and `xpcall` functions. Please upgrade your database before running RLS!
 
@@ -14,6 +14,14 @@ We also added validation for Virtual Schema properties to all requests that use
 
 Push-down of aggregate functions is not yet implemented. We will add that in a future version (issue [#120](https://github.com/exasol/row-level-security-lua/issues/120)).
 
+## Features
+
+* #118: Added support for `LIMIT` and `GROUP BY` via update of `virtual-schema-common-lua`
+
+## Refactoring
+
+* #122: Applied new object-oriented style 
+
 ## Features 
 
 * #118: Added support for `ORDER BY`, `LIMIT` more scalar functions

pom.xml

@@ -204,6 +204,19 @@
  </execution>
  </executions>
  </plugin>
+ <plugin>
+ <groupId>com.exasol</groupId>
+ <artifactId>error-code-crawler-maven-plugin</artifactId>
+ <version>1.1.1</version>
+ <executions>
+ <execution>
+ <!-- Prevent the crawler from running, Java is only used for tests. -->
+ <id>verify</id>
+ <goals/>
+ <phase/>
+ </execution>
+ </executions>
+ </plugin>
  <!-- Since this is a Lua project, no production JAR should be created. Empty JAR gets skipped. -->
  <plugin>
  <groupId>org.apache.maven.plugins</groupId>

row-level-security-lua-1.2.0-1.rockspec

@@ -22,7 +22,7 @@ description = {
 }
 
 dependencies = {
- "virtual-schema-common-lua = 1.1.0"
+ "virtual-schema-common-lua = 2.0.0"
 }
 
 build_dependencies = {
@@ -46,9 +46,10 @@ local package_items = {
  "message_expander",
  -- from virtual-schema-common-lua"
  "exasolvs.AbstractVirtualSchemaAdapter", "exasolvs.AdapterProperties", "exasolvs.RequestDispatcher",
- "exasolvs.Query", "exasolvs.QueryRenderer", "exasolvs.queryrenderer.AbstractQueryAppender",
- "exasolvs.queryrenderer.ExpressionAppender", "exasolvs.queryrenderer.ScalarFunctionAppender",
- "exasolvs.queryrenderer.SelectAppender", "text"
+ "exasolvs.Query", "exasolvs.QueryRenderer",
+ "exasolvs.queryrenderer.AbstractQueryAppender", "exasolvs.queryrenderer.ExpressionAppender",
+ "exasolvs.queryrenderer.ScalarFunctionAppender", "exasolvs.queryrenderer.SelectAppender",
+ "text"
 }
 
 local item_path_list = ""

spec/entry.spec.lua → spec/entry_spec.lua

@@ -3,11 +3,6 @@ require("busted.runner")()
 require("entry")
 
 describe("Entry script", function()
- it("handles an adapter call to drop the virtual schema", function()
- local result = _G.adapter_call([[{"type" : "dropVirtualSchema"}]])
- assert.are.equal([[{"type":"dropVirtualSchema"}]], result)
- end)
-
  local function create_exasol_context_stub()
  return {
  pquery_no_preprocessing = function(query)
@@ -20,8 +15,14 @@ describe("Entry script", function()
  }
  end
 
+ _G.exa = create_exasol_context_stub()
+
+ it("handles an adapter call to drop the virtual schema", function()
+ local result = _G.adapter_call([[{"type" : "dropVirtualSchema"}]])
+ assert.are.equal([[{"type":"dropVirtualSchema"}]], result)
+ end)
+
  it("handles an adapter call to create the virtual schema", function()
- _G.exa = create_exasol_context_stub()
  local result = _G.adapter_call(
  [[{"type" : "createVirtualSchema",
  "schemaMetadataInfo" : {

spec/exasolrls/RlsAdapterProperties_spec.lua

@@ -20,7 +20,7 @@ describe("adapter_properties", function()
  }
  for _, test in ipairs(tests) do
  it(test.expected, function()
- local properties = RlsAdapterProperties.create(test.properties)
+ local properties = RlsAdapterProperties:new(test.properties)
  assert.error_matches(function () properties:validate() end, test.expected, 1, true)
  end)
  end

spec/exasolrls/RlsAdapter_spec.lua

@@ -14,7 +14,7 @@ describe("RlsAdapter", function()
 
  before_each(function()
  metadata_reader_mock = mockagne.getMock()
- adapter = RlsAdapter.create(metadata_reader_mock)
+ adapter = RlsAdapter:new(metadata_reader_mock)
  properties_stub = {
  get_table_filter = function() return {} end,
  has_excluded_capabilities = function() return false end
@@ -46,7 +46,7 @@ describe("RlsAdapter", function()
  end)
 
  it("confirms a request to drop the Virtual Schema with an empty response", function()
- assert.are.same({type = "dropVirtualSchema"}, RlsAdapter.drop_virtual_schema())
+ assert.are.same({type = "dropVirtualSchema"}, adapter:drop_virtual_schema())
  end)
 
  it("reports the supported capabilities", function()

spec/smoke_test_spec.lua

@@ -23,26 +23,6 @@ local function load_distribution()
  end
 end
 
---local function search_ignoring_dots(module_name)
--- local message = {}
--- print("no dot searcher: " .. package.path)
--- for path in string.gmatch(package.path, "[^;]+") do
--- local module_path = string.gsub(path, "?", module_name)
--- print("Checking " .. module_path)
--- local file <close> = io.open(module_path)
--- if file then
--- return module_path
--- else
--- table.insert(message, "\n\tno file (..) '" .. module_path .. "'")
--- end
--- end
--- return nil, table.concat(message)
---end
---
---table.insert(package.searchers, 2, search_ignoring_dots)
---
---local distribution = require(DISTRIBUTION_MODULE .. "-" .. VERSION)
-
 if load_distribution() then
  describe("Distribution (smoke test)", function()
  it("handles an adapter call to drop the virtual schema", function()

src/main/lua/administration/exa_identifier.lua

@@ -8,13 +8,13 @@ local ALLOWED_IDENTIFIER = "Allowed identifiers are ASCII only, starting with a
 ---
 -- Validate an Exasol identifier.
 -- <p>
--- Note that string parameters passed to Exasol scripts can never be empty. Exasol replaces empty <code>VARCHAR</code>
--- values by <code>NULL</code>. So if an identifier is stored or passed as as <code>VARCHAR</code> this rule applies.
+-- Note that string parameters passed to Exasol scripts can never be empty. Exasol replaces empty `VARCHAR`
+-- values by `NULL`. So if an identifier is stored or passed as as `VARCHAR` this rule applies.
 -- </p>
 --
--- @param identifer identifier to be validated against the rules for being a valid Exasol identifier
+-- @param identifier identifier to be validated against the rules for being a valid Exasol identifier
 --
--- @return <code>true</code> if the identifier is valid. Otherwise return false and a quoted identifier for use in an
+-- @return `true` if the identifier is valid. Otherwise return false and a quoted identifier for use in an
 -- error message
 --
 function validate(identifier)
@@ -32,7 +32,7 @@ function validate(identifier)
 end
 
 ---
--- Add quotes around a string and display <code>nil</code> and <code>null</code> in a clear fashion.
+-- Add quotes around a string and display `nil` and `null` in a clear fashion.
 --
 -- @param string string to be quoted
 --

src/main/lua/administration/exa_rls_base.lua

@@ -31,7 +31,7 @@ end
 --
 -- @param role_name name of the role
 --
--- @return ID of the role or <code>nil</code> if the role does not exist
+-- @return ID of the role or `nil` if the role does not exist
 --
 function get_role_id_by_name(role_name)
  res = query([[SELECT role_id FROM ::s.EXA_ROLES_MAPPING WHERE upper(role_name) = upper(:r)]],
@@ -48,7 +48,7 @@ end
 --
 -- @param role_id ID of the role
 --
--- @return name of the role or <code>nil</code if the role does not exist
+-- @return name of the role or `nil` if the role does not exist
 --
 function get_role_name_by_id(role_id)
  res = query([[SELECT role_name FROM ::s.EXA_ROLES_MAPPING WHERE role_id = :i]],

src/main/lua/entry.lua

@@ -1,25 +1,18 @@
----
--- This is the main entry point of the Lua Virtual Schema adapter.
--- <p>
+--- Main entry point of the Lua Virtual Schema adapter.
 -- It is responsible for creating and wiring up the main adapter objects.
--- </p>
 
 local RlsAdapter = require("exasolrls.RlsAdapter")
 local RlsAdapterProperties = require("exasolrls.RlsAdapterProperties")
 local MetadataReader = require("exasolrls.MetadataReader")
 local RequestDispatcher = require("exasolvs.RequestDispatcher")
 
----
--- Handle a Virtual Schema request.
---
+--- Handle a Virtual Schema request.
 -- @param request_as_json JSON-encoded adapter request
---
 -- @return JSON-encoded adapter response
---
 function adapter_call(request_as_json)
  local exasol_context = _G.exa
  local metadata_reader = MetadataReader:new(exasol_context)
- local adapter = RlsAdapter.create(metadata_reader)
- local dispatcher = RequestDispatcher.create(adapter, RlsAdapterProperties)
+ local adapter = RlsAdapter:new(metadata_reader)
+ local dispatcher = RequestDispatcher:new(adapter, RlsAdapterProperties)
  return dispatcher:adapter_call(request_as_json)
-end
+end

src/main/lua/exasolrls/MetadataReader.lua

@@ -7,15 +7,21 @@ local DEFAULT_SRID <const> = 0
 --- This class reads schema, table and column metadata from the source.
 -- @type MetadataReader
 local MetadataReader = {}
+MetadataReader.__index = MetadataReader
 
---- Create a new <code>MetadataReader<code>.
--- @param exasol_context pre-initialized instance
+--- Create a new `MetadataReader`.
+-- @param exasol_context handle to local database functions and status
 -- @return metadata reader
 function MetadataReader:new(exasol_context)
- local object = {exasol_context = exasol_context}
- self.__index = self
- setmetatable(object, self)
- return object
+ assert(exasol_context ~= nil,
+ "The metadata reader requires an Exasol context handle in order to read metadata from the database")
+ local instance = setmetatable({}, self)
+ instance:_init(exasol_context)
+ return instance
+end
+
+function MetadataReader:_init(exasol_context)
+ self._exasol_context = exasol_context
 end
 
 function MetadataReader:_translate_parameterless_type(column_id, column_type)
@@ -113,7 +119,7 @@ end
 function MetadataReader:_translate_columns_metadata(schema_id, table_id)
  local sql = '/*snapshot execution*/ SELECT "COLUMN_NAME", "COLUMN_TYPE" FROM "SYS"."EXA_ALL_COLUMNS"'
  .. ' WHERE "COLUMN_SCHEMA" = :s AND "COLUMN_TABLE" = :t'
- local ok, result = self.exasol_context.pquery_no_preprocessing(sql, {s = schema_id, t = table_id})
+ local ok, result = self._exasol_context.pquery_no_preprocessing(sql, {s = schema_id, t = table_id})
  local translated_columns = {}
  local tenant_protected, role_protected, group_protected
  if ok then
@@ -184,7 +190,7 @@ end
 
 function MetadataReader:_translate_table_metadata(schema_id, include_tables)
  local sql = '/*snapshot execution*/ SELECT "TABLE_NAME" FROM "SYS"."EXA_ALL_TABLES" WHERE "TABLE_SCHEMA" = :s'
- local ok, result = self.exasol_context.pquery_no_preprocessing(sql, {s = schema_id})
+ local ok, result = self._exasol_context.pquery_no_preprocessing(sql, {s = schema_id})
  if ok then
  return self:_translate_table_scan_results(schema_id, result, include_tables)
  else

src/main/lua/exasolrls/QueryRewriter.lua

@@ -4,9 +4,7 @@ local log = require("remotelog")
 local exaerror = require("exaerror")
 
 --- This class rewrites the query, adding RLS protection if necessary.
---
--- @type QueryRewriter
---
+-- @classmod QueryRewriter
 local QueryRewriter = {}
 
 local function validate(query)