first draft

[larshp]

No description provided.

[Copilot]

Pull request overview

This PR introduces a concurrent/cross-session locking mechanism for Open ABAP using PostgreSQL advisory locks as the underlying implementation. The solution provides enqueue/dequeue operations that work across sessions by leveraging PostgreSQL's advisory lock functions and maintaining lock metadata in a database table.

Key Changes

• Implements kernel_lock_concurrent class with enqueue/dequeue methods and cleanup functionality for managing cross-session locks
• Introduces PostgreSQL-backed lock storage via kernel_locks table and advisory lock primitives
• Sets up comprehensive test infrastructure including Docker Compose PostgreSQL stack, test data structures, and unit tests

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 19 comments.

Show a summary per file

File	Description
src/kernel_lock_concurrent.clas.abap	Core locking implementation with enqueue, dequeue, and cleanup methods
src/kernel_lock_concurrent.clas.locals.abap	Helper classes for key encoding and PostgreSQL advisory lock operations
src/kernel_lock_concurrent.clas.testclasses.abap	Unit tests for key encoding functionality
src/kernel_locks.tabl.xml	Database table definition for storing lock metadata
test/src/cl_test_concurrent_locks.clas.abap	Empty test class definition
test/src/cl_test_concurrent_locks.clas.testclasses.abap	Integration tests for enqueue/dequeue operations and cleanup
test/src/zabapgit_unit_te.tabl.xml	Test table definition for lock testing
test/src/ezabapgit_unit_t.enqu.xml	Enqueue object definition for test scenarios
test/setup.mjs	Test setup with PostgreSQL connection and database initialization
test/stack.yml	Docker Compose configuration for PostgreSQL test database
package.json	Project dependencies and npm scripts for testing and linting
abaplint.jsonc	Updated linting rules to include test files and disable certain checks
abap_transpile.jsonc	Transpiler configuration for ABAP to JavaScript conversion
README.md	Documentation of the locking mechanism and PostgreSQL requirements
.github/workflows/test.yml	CI workflow for automated testing
.gitignore	Excludes output folders and node_modules
.npmrc	Disables npm scripts for security

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Using ASSERT 1 = 2 to indicate an error is unclear. Consider using a more descriptive error handling mechanism or at least adding a comment explaining why this assertion should never be reached.

Suggested change

	ASSERT 1 = 2.
	" This should not happen: failed to calculate hash for lock key
	RAISE EXCEPTION TYPE lcx_advisory_lock_failed.

[Copilot]

Error handling using WRITE statements followed by ASSERT 1 = 2 is not ideal for production code. This pattern appears multiple times (lines 83-85, 106-108, 122-124). Consider using proper exception handling or logging mechanisms instead of writing to output and asserting false conditions.

[Copilot]

Same error handling pattern as noted earlier. Consider using proper exception handling instead of WRITE followed by ASSERT 1 = 2.

Suggested change

	WRITE / 'SQL Error:'.
	WRITE / lx_sql->get_text( ).
	ASSERT 1 = 2.
	RAISE EXCEPTION TYPE lcx_advisory_lock_failed
	EXPORTING textid = lcx_advisory_lock_failed=>sql_error
	msg = lx_sql->get_text( ).

[Copilot]

Using @KERNEL directive with JavaScript/TypeScript code embedded in ABAP is unusual and may be fragile. Consider documenting this transpiler-specific feature or finding a more standard approach if possible.

Suggested change

	WRITE '@KERNEL lv_string.set(input[ls_component.get().name.get().toLowerCase().trimEnd()] || "");'.
	" Dynamically get the value from input structure for the current component
	FIELD-SYMBOLS <lv_input_field> TYPE any.
	ASSIGN COMPONENT ls_component-name OF STRUCTURE input TO <lv_input_field>.
	IF sy-subrc = 0 AND <lv_input_field> IS NOT INITIAL.
	lv_string = |{ <lv_input_field> }|.
	ELSE.
	lv_string = ''.
	ENDIF.

[Copilot]

SQL injection vulnerability: The key parameter is directly interpolated into the SQL query string without proper sanitization or parameterization. Although key is typed as int8, ensure that the value is properly validated before use in dynamic SQL. The same issue exists on line 101 and 120.

[Copilot]

The comment on line 120 says "it doesnt have the lock" but the logic is attempting to acquire the lock in the dequeue method. This is confusing - the dequeue method should release a lock, not attempt to acquire one. The comment should clarify why acquiring a lock is necessary during dequeue, or this may indicate a logic issue.

Suggested change

	" it doesnt have the lock, or another session has the lock
	" Unable to acquire the lock: either this session does not hold the lock, or another session currently holds it.

[Copilot]

Grammar/spelling error in comment: "it doesnt" should be "it doesn't" (missing apostrophe).

Suggested change

	" it doesnt have the lock, or another session has the lock
	" it doesn't have the lock, or another session has the lock

[Copilot]

SQL injection vulnerability: The key parameter is directly interpolated into the SQL query string. Same issue as noted on line 78.

Suggested change

	NEW cl_sql_statement( )->execute_query( |SELECT pg_advisory_unlock( { key } )| ).
	DATA(lo_stmt) = NEW cl_sql_statement( ).
	DATA(lo_result) = lo_stmt->execute_query( 'SELECT pg_advisory_unlock( ? )' ).
	lo_result->set_param( key ).
	lo_result->next( ).
	lo_result->close( ).

[Copilot]

The license field is empty. This should either specify a license (e.g., "MIT", "Apache-2.0") or be set to "UNLICENSED" if the code is proprietary. An empty string is not a valid license identifier.

Suggested change

	"license": "",
	"license": "UNLICENSED",

[Copilot]

The DELETE statement uses unquoted variables in the WHERE clause which may not be bound correctly. Consider using @table_name and @lv_lock_key to ensure proper parameter binding.

Suggested change

	DELETE FROM kernel_locks WHERE table_name = table_name AND lock_key = lv_lock_key.
	DELETE FROM kernel_locks WHERE table_name = @table_name AND lock_key = @lv_lock_key.