ISSUE-680 # Add the DB SQL Executor to import data from CSV and execute SQL statements

[javiertuya]

Add the DB SQL Executor to import data from CSV and execute SQL statements

Fixes Issue

• [x] Which issue or ticket was(will be) fixed by this PR? Close #680

PR Branch https://github.com/javiertuya/zerocode/tree/680-db-sql-executor

Documentation at PR https://github.com/authorjapps/zerocode-tdd-docs/pull/24

Motivation and Context

This PR provides an out of the box component that uses the Java API to execute steps that interact with a database.

The executor class org.jsmart.zerocode.core.db.DbSqlExecutor provides two operations:

• EXECUTE: Run a SQL statement to verify the data stored in the database or insert/update in the data.
• LOADCSV: Load the contents of a CSV file located in the resources folder into a table.

Acceptance critera fulfillment:

• AC1, AC2, AC5, AC6: Done, with unit and integration tests, H2 database.
• AC3: Done, PostgreSQL: the same unit and integration tests were executed in local, not in CI
• AC4: Partially done, read CSV files from src/test/resources. Reading files from URL to be implemented in separate issue

Suggestions for future improvements:

• Load from an url (AC4)
• Load from sites that require authentication (mentioned in the issue)
• Load from anywhere in the filesystem
• Custom datetime formats (specified in the properties file or in the request)
• Load data into binary columns (e.g. hexadecimal, base64...)
• Run the PostgreSQL tests as part of the GitHub Actions workflow

Checklist:

• [x] New Unit tests were added

  • [ ] Covered in existing Unit tests

• [x] Integration tests were added

  • [ ] Covered in existing Integration tests

• [x] Test names are meaningful

• [x] Feature manually tested and outcome is successful

• [x] PR doesn't break any of the earlier features for end users

  • [ ] WARNING! This might break one or more earlier earlier features, hence left a comment tagging all reviewrs

• [x] Branch build passed in CI

• [x] No 'package.*' in the imports

• [x] Relevant DOcumentation page added or updated with clear instructions and examples for the end user

  • [ ] Not applicable. This was only a code refactor change, no functional or behaviourial changes were introduced

• [ ] Http test added to http-testing module(if applicable) ?

  • [x] Not applicable. The changes did not affect HTTP automation flow

• [ ] Kafka test added to kafka-testing module(if applicable) ?

  • [x] Not applicable. The changes did not affect Kafka automation flow

[nirmalchandra]

@javiertuya , excellent work! I've given the CI run and awaiting for a Green(check status here).

Just checking:

1. Did you get chance to run a Integration-test scenario against a real Postgres DB (as the CI tests are using h2db) ?
   • Even Docker should be fine I guess, as this will validate the JDBC driver aspects ok
2. Is it possible for you(if you by chance have an Infra with any DB) to run against a remote DB instance (example: RDS ) to check the Drivers work as expected?

[javiertuya]

@nirmalchandra

1. Did you get chance to run a Integration-test scenario against a real Postgres DB (as the CI tests are using h2db) ?

Yes. The target DB is configured in core/src/test/resources/db_test.properties (this file has some instructions embedded). I ran both unit and integration tests using three different versions of Postgres. The only difference is that Postgress stores column and table identifiers as lowercase, so that, to make the integration tests pass, you need convert the json scenarios to have the key values in the verify to lowercase (this is done from the junit in the case of unit tests). For example, to run the integration test that loads a table from a CSV file with headers, you can do this case conversion by running from the db subfolder:

sed -i 's/"ID"/"id"/' db_csv_load_with_headers.json
sed -i 's/"NAME"/"name"/' db_csv_load_with_headers.json
sed -i 's/"AGE"/"age"/' db_csv_load_with_headers.json

• Even Docker should be fine I guess, as this will validate the JDBC driver aspects ok

It would also be possible to run the postgres tests in the Github Actions (spin-up postgres takes around 6 seconds)

2. Is it possible for you(if you by chance have an Infra with any DB) to run against a remote DB instance (example: RDS ) to check the Drivers work as expected?

I just checked it: connecting from a desktop to a remote server in my lab (through a VPN) and it works fine. To check this, just run a postgress instance in the server, and form the desktop, change the db.driver.url property to put the server ip and port

[authorjapps]

@a1shadows , @omkar-shitole 👋 : Can you guys also have a look at the PR and review it? Also if possible, check against some other real DB e.g. real MySQL etc?

[a1shadows]

Should we also try and create some documentation to https://github.com/authorjapps/zerocode-tdd-docs/tree/main? Otherwise, feature looks good to me. I'll try and run it on my local this weekend

[authorjapps]

Should we also try and create some documentation to https://github.com/authorjapps/zerocode-tdd-docs/tree/main? Otherwise, feature looks good to me. I'll try and run it on my local this weekend

@a1shadows , It's already here.

But, please check if you want to add anything to it or amend it to make it more helpful, then please proceed 👍

[javiertuya]

@a1shadows , @omkar-shitole 👋 : Can you guys also have a look at the PR and review it? Also if possible, check against some other real DB e.g. real MySQL etc?

I've tested against MySQL. To run the tests, it needs a few changes:

• A customization in tests due to MySQL particularities (e.g. ORDER BY ID NULLS FIRST must be made differently)
• Test database setup: MySQL driver does not support running multiple SQLs (separted by ;) in a single statement. This will require (1) to separate the setup SQL statements, and, (2) allow an array of SQLs in the EXECUTE operation to perform all setup in a single step.

I can do the above changes in new commits in this PR or after review and merge, as you prefer.

[authorjapps]

@javiertuya , I ran this against a Postgres DB. It didn't execute as expected. Can you have a look and fix it if possible?

Looks like it is expecting a correct Driver.

Note: I didn't create any tables yet, just wanted to see if the SQL gets executed or not.

Docker PG brought up from here: Branch:

pr_686_db_sql/zerocode/docker/compose/pg_compose.yml

I ran this

src/test/resources/integration_test_files/db/db_sql_execute.json

Config: test/resources/db_test.properties :

db.driver.url=jdbc:postgresql://localhost:35432/postgres
db.driver.user=postgres
db.driver.password=example

Error:

2024-11-20 00:06:24,453 [main] ERROR org.jsmart.zerocode.core.db.DbSqlExecutor - Failed to create connection, Please check the target environment properties to connect the database (db.driver.url, db.driver.user and db.driver.password)

Fix:

Uncommenting this in the POM.xml started working.

<dependency>
		    <groupId>org.postgresql</groupId>
		    <artifactId>postgresql</artifactId>
		    <version>42.7.4</version>
		    <scope>test</scope>
</dependency>

Action:

Can you keep the driver uncommented, and enable couple of (1 or 2 tests) for CI to run against PG DB? Better add another Test file(e.g. PostgresCITest.java) -> you will have to bring up the PG via the docker-compose in the actions yaml file, then fire these tests

[javiertuya]

@authorjapps Done. A new commit (https://github.com/authorjapps/zerocode/pull/686/commits/3ca2575ddac828dd36fbf88fb930d2d53955394b) that:

• Adds the new postgres test that runs two scenarios in CI (load csv with headers and execute sql).
• Adds the properties file (pg driver configuration) and the two json scenarios (as postgres stores identifiers in lowercase, they are a copy of the corresponding H2 tests, with the assert keys in lowercase).
• Naming convention: files related to the pg tests add the word postgres as a suffix.
• Comments updated in other files referring to running the postgres tests.

[authorjapps]

We may not need this in test scope. We need this Driver version 42.7.4 be available as default in the dependant projects. (Whichever project needs a different version, they can override in their POM file.)

Driver <postgres.db.version>42.7.4</postgres.db.version> ==> runs fine against postgres:9.3 Db (Let me create a table to list this Postgres DB vs Driver compatible, which might be useful later on.

<dependency> 
                <groupId>org.postgresql</groupId>
                <artifactId>postgresql</artifactId>
                <version>${postgres.db.version}</version>
<!--                <scope>test</scope>-->
</dependency>

[javiertuya]

@authorjapps @nirmalchandra This PR is approved and merged 👋, but there is an unresolved comment: https://github.com/authorjapps/zerocode/pull/686#discussion_r1792532640. Do you still want me to move the integration tests as indicated in this comment?

[authorjapps]

@javiertuya , yes please, appreciate you spotted this. Thanks. A new PR would should do.

Cheers