Merge pull request #3186 from fhanik/pr/fhanik-db-tests

Fix DB unit tests

README.md

@@ -209,14 +209,33 @@ You can run the integration tests with docker
 
 will create a docker container running uaa + ldap + database whereby integration tests are run against.
 
-### Using Gradle to test with postgresql or mysql
+### Using Docker to test with postgresql or mysql
 
 The default uaa unit tests (./gradlew test integrationTest) use hsqldb.
 
 To run the unit tests with docker:
 
     $ run-unit-tests.sh <dbtype>
 
+### Using Gradle to test with Postgres or MySQL
+
+You need a locally running database. You can launch a Postgres 15 and MySQL 8 locally with docker compose:
+
+    $ docker compose --file scripts/docker-compose.yaml up
+
+If you wish to launch only one of the DBs, select the appropriate service name:
+
+    $ docker compose --file scripts/docker-compose.yaml up postgresql
+
+Then run the test with the appropriate profile:
+
+    $ ./gradlew '-Dspring.profiles.active=postgresql,default' \
+        --no-daemon \
+        test
+
+There are special guarantees in place to avoid pollution between tests, so be sure to run the images
+from the compose script, and run your test with `--no-daemon`. To learn more, read [docs/testing.md](docs/testing.md).
+
 ### To run a single test
 
 The default uaa unit tests (`./gradlew test`) use hsqldb. 

docs/testing.md

@@ -0,0 +1,136 @@
+# Testing considerations
+
+This document contains information about how the tests are structured and why.
+
+## Types of test
+
+There are two types of tests:
+
+- Unit tests, which run tests on classes or subsets of the application in a single JVM. Those run with `./gradlew test`.
+- Integration tests, which launch the UAA application and run web-based tests the running app. Those can be run with
+  `./gradlew integrationTest`
+
+## Helper scripts
+
+There are helper scripts, `run-unit-tests.sh` and `run-integration-tests.sh`, which run the tests inside a docker
+container. The docker container they run in contains the database against which to run the tests, as well as an LDAP
+server. It is self-contained but lacks flexibility. It relies on custom-baked image that may not support arm64, and
+can't work with your IDE.
+
+However, since the scripts run a container with all dependencies, you do not need infrastructure to run against a
+specified DB:
+
+    $ run-unit-tests.sh <dbtype>
+
+## Test databases
+
+By default, the tests run against an in-memory DB, `hsqldb`. This DB is also present in the prod artifact, so that
+UAA can also be ran standalone to test tweaks in a live instance.
+
+To run these databases locally, use the docker-compose script:
+
+    $ docker compose --file scripts/docker-compose.yaml up
+
+If you wish to launch only one of the DBs, select the appropriate service name:
+
+    $ docker compose --file scripts/docker-compose.yaml up postgresql
+
+To run tests against either Postgres or MySQL, use the `postgresql` or `mysql` profile, to select the DB. Be sure
+to add the `default` profile which will trigger seeding the database with some admin users, clients, etc. For example:
+
+    $ ./gradlew '-Dspring.profiles.active=mysql,default test
+
+To run tests from your IDE against a given database, you can (temporarily) annotate the test class:
+
+```java
+
+@ActiveProfiles({"mysql", "default"})
+class MyCustomTests {
+    @Test
+    void foo() {
+        // ...
+    }
+}
+```
+
+## Database-specific tests
+
+Some tests only work on a single type of database, for example `MySqlDbMigrationIntegrationTest`; or do not work on a
+given database, for example `JdbcClientMetadataProvisioningTest.createdByPadsTo36Chars`. You can turn tests on and off
+based on the profile with custom annotations, `@DisabledIfProfile` and `@EnabledIfProfile`, for example:
+
+```java
+// Only run on either mysql or postgresql
+@EnabledIfProfile({"mysql", "postgresql"})
+class RealDbOnlyTests {
+
+}
+
+// or:
+
+class SomeTests {
+
+    // Do not run when there is either mysql or postgresql
+    @DisabledIfProfile({"mysql", "postgres"})
+    void notOnRealDb() {
+
+    }
+
+}
+```
+
+## Test pollution
+
+There might be test pollution when tests are run in parallel, or even between projects. For example, when you run
+
+    $ ./gradlew test
+
+It will run tests in both `cloudfoundry-identity-server` and `cloudfoundry-identity-uaa` projects. Both need a database,
+and both do sometimes clean up the database.
+
+To avoid test pollution, 24 databases are created, and each Gradle "worker" thread gets its own database. A Gradle
+worker has a numeric `id`, and each time a new task is spun up, the idea of the worker picking up the task increases.
+So there are 24 DBs with names `uaa_1`, `uaa_2`, ... created, and usually the worker ID stays below 24 and there are
+enough databases for each test.
+
+However, if the gradle daemon is kept running in the background and is re-used for subsequent tasks, e.g. by doing:
+
+    $ ./gradlew test # first run
+    # do some code changes
+    $ ./gradlew test
+
+You will get new workers with IDs > 24. It is recommended you run your Gradle in no-daemon mode when running tests:
+
+    $ ./gradlew test --no-daemon
+
+It will be slightly slower to start up (a few seconds), but the tests take multiple minutes and so the gain of using
+a daemon is not worth the trouble.
+
+## Timezone issues
+
+The UAA and its DB server _MUST_ have the same timezone, because dates are not uniformly stored in UTC and timezones
+do matter. Specifically for MySQL, there are issues when your local host is ahead of UTC, because:
+
+1. The default containers runs in UTC
+2. When calling `current_timestamp` the value is in UTC
+3. But when calling a prepared statement from JDBC with a Date/Timestamp/time-based the timezone is sent to the server
+
+So, when running e.g. in `Europe/Paris` (CET):
+
+```java
+jdbcTemplate.queryForObject("SELECT CURRENT_TIMESTAMP",String .class);
+// will return 15:00UTC
+// if the TZ is dropped, it is recorded as 15:00
+jdbcTemplate.update("UPDATE foo SET updated=?",new Date(System.currentTimeMillis()));
+// will insert 16:00CET
+// if the TZ is dropped this is recorded as 16:00
+```
+
+For this reason, we update the MySQL container in `docker-compose.yml` to have the same timezone as the host through
+the `$TZ` env var.
+
+If you have timing-based issues in your test, ensure that you set `$TZ` before starting docker compose, e.g.:
+
+    $ TZ="Europe/Paris" docker compose up
+
+It is not required, and MySQL will default to using UTC.

scripts/docker-compose.yml

@@ -0,0 +1,50 @@
+name: uaa
+
+services:
+  postgres:
+    image: "postgres:15"
+    ports:
+      - 5432:5432
+    volumes:
+      - ./postgresql:/docker-entrypoint-initdb.d/
+      - type: tmpfs
+        target: /var/lib/postgresql/data
+        tmpfs:
+          size: 1073741824 # 1024 * 2^20 bytes = 1024Mb
+    environment:
+      - POSTGRES_PASSWORD=changeme
+  mysql:
+    image: "mysql:8"
+    ports:
+      - 3306:3306
+    volumes:
+      - ./mysql:/docker-entrypoint-initdb.d/
+      - /etc/localtime:/localtime-from-host
+      - type: tmpfs
+        target: /var/lib/mysql
+        tmpfs:
+          size: 1073741824 # 1024 * 2^20 bytes = 1024Mb
+    environment:
+      - MYSQL_ROOT_PASSWORD=changeme
+      - TZ=${TZ}
+    command:
+      - --sql_mode=ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION,PAD_CHAR_TO_FULL_LENGTH
+  openldap:
+    image: docker.io/bitnami/openldap:2.6
+    ports:
+      - '389:1389'
+      - '636:1636'
+    # docs of these env vars: https://github.com/bitnami/containers/tree/2724f9cd02b3b4e7986a1e2a0b0b30af3737bbd2/bitnami/openldap#configuration
+    environment:
+      - LDAP_ROOT=dc=test,dc=com
+      - LDAP_ADMIN_USERNAME=admin
+      - LDAP_ADMIN_PASSWORD=password
+      - LDAP_USERS=user01,user02
+      - LDAP_PASSWORDS=password1,password2
+      - LDAP_GROUP=some-ldap-group
+    volumes:
+      - 'openldap_data:/bitnami/openldap'
+
+volumes:
+  openldap_data:
+    driver: local

scripts/mysql/init-db.sh

@@ -0,0 +1,50 @@
+#!/usr/bin/env bash
+
+set -euo pipefail
+
+# Number of gradle workers times 4, which was somewhat arbitrary but is sufficient in practice.
+# We make extra dbs because a gradle worker ID can exceed the max number of workers.
+NUM_OF_DATABASES_TO_CREATE=24
+
+function initDB() {
+  mysql -uroot -pchangeme <<-EOSQL
+  SET GLOBAL max_connections = 250;
+  DROP DATABASE IF EXISTS uaa;
+  CREATE DATABASE uaa DEFAULT CHARACTER SET utf8mb4;
+EOSQL
+}
+
+function createDB() {
+  DATABASE_NAME="uaa_${1}"
+  echo "Creating MySQL database with name ${DATABASE_NAME}"
+  mysql  -uroot -pchangeme <<-EOSQL
+    DROP DATABASE IF EXISTS $DATABASE_NAME;
+    CREATE DATABASE $DATABASE_NAME DEFAULT CHARACTER SET utf8mb4;
+EOSQL
+}
+
+function setTimezone() {
+  # If the "TZ" env var is set in the container definition, then set the
+  # DB at the given timezone. When "TZ" is unset, use the DB default (UTC).
+  #
+  # This is important because the database should run in the same timezone as the UAA,
+  # and, in the case of tests, the same timezone as the JVM running the tests.
+  #
+  # We achieve consistency by changing the timezone inside the DB; because setting
+  # it in the container is complicated. The container is missing the `timedatectl`
+  # binary ; and the script runs as the mysql user which does not have sudo privileges.
+  if [[ -n "$TZ" ]]; then
+    echo "Setting DB timezone to: $TZ"
+    mysql -uroot -pchangeme <<-EOSQL
+    SET GLOBAL time_zone = "$TZ";
+EOSQL
+  fi
+}
+
+
+
+initDB
+
+for db_id in `seq 1 $NUM_OF_DATABASES_TO_CREATE`; do
+  createDB $db_id
+done

scripts/postgresql/init-db.sh

@@ -0,0 +1,32 @@
+#!/usr/bin/env bash
+
+set -euo pipefail
+
+# Number of gradle workers times 4, which was somewhat arbitrary but is sufficient in practice.
+# We make extra dbs because a gradle worker ID can exceed the max number of workers.
+NUM_OF_DATABASES_TO_CREATE=24
+
+function initDB() {
+  psql --username "$POSTGRES_USER" --dbname "$POSTGRES_DB" <<-EOSQL
+    DROP DATABASE IF EXISTS uaa;
+    CREATE DATABASE uaa;
+    DROP USER IF EXISTS root;
+    CREATE USER root WITH SUPERUSER PASSWORD '$POSTGRES_PASSWORD';
+    SHOW max_connections;
+EOSQL
+}
+
+function createDB() {
+  DATABASE_NAME="uaa_${1}"
+  echo "Creating PostgreSQL database with name ${DATABASE_NAME}"
+  psql --username "$POSTGRES_USER" --dbname "$POSTGRES_DB" <<-EOSQL
+    DROP DATABASE IF EXISTS $DATABASE_NAME;
+    CREATE DATABASE $DATABASE_NAME;
+EOSQL
+}
+
+initDB
+
+for db_id in `seq 1 $NUM_OF_DATABASES_TO_CREATE`; do
+  createDB $db_id
+done

server/src/main/java/org/cloudfoundry/identity/uaa/db/beans/FlywayConfiguration.java

@@ -63,6 +63,9 @@ public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata)
         public Flyway flyway(Flyway baseFlyway) {
             baseFlyway.repair();
             baseFlyway.migrate();
+            org.apache.tomcat.jdbc.pool.DataSource ds =
+                    (org.apache.tomcat.jdbc.pool.DataSource)baseFlyway.getConfiguration().getDataSource();
+            ds.purge();
             return baseFlyway;
         }
     }

server/src/main/java/org/cloudfoundry/identity/uaa/db/mysql/V4_9_2__AddPrimaryKeysIfMissing.java

@@ -4,6 +4,10 @@
 import org.flywaydb.core.api.migration.Context;
 import org.springframework.jdbc.core.JdbcTemplate;
 import org.springframework.jdbc.datasource.SingleConnectionDataSource;
+import org.springframework.jdbc.datasource.lookup.DataSourceLookupFailureException;
+
+import java.sql.Connection;
+import java.sql.SQLException;
 
 
 public class V4_9_2__AddPrimaryKeysIfMissing extends BaseJavaMigration {
@@ -16,11 +20,19 @@ public void migrate(Context context) throws Exception {
         JdbcTemplate jdbcTemplate = new JdbcTemplate(new SingleConnectionDataSource(
                 context.getConnection(), true));
         for (String table : tables) {
-            int count = jdbcTemplate.queryForObject(checkPrimaryKeyExists, Integer.class, jdbcTemplate.getDataSource().getConnection().getCatalog(), table);
+            int count = jdbcTemplate.queryForObject(checkPrimaryKeyExists, Integer.class, getDatabaseCatalog(jdbcTemplate), table);
             if (count == 0) {
                 String sql = "ALTER TABLE " + table + " ADD COLUMN `id` int(11) unsigned PRIMARY KEY AUTO_INCREMENT";
                 jdbcTemplate.execute(sql);
             }
         }
     }
+
+    private String getDatabaseCatalog(JdbcTemplate jdbcTemplate) {
+        try (Connection connection = jdbcTemplate.getDataSource().getConnection()) {
+            return  connection.getCatalog();
+        } catch (SQLException e) {
+            throw new DataSourceLookupFailureException("Unable to look up database schema.", e);
+        }
+    }
 }

server/src/main/resources/org/cloudfoundry/identity/uaa/db/hsqldb/V4_112__Users_id_to_Varchar_MySQL.sql

@@ -0,0 +1 @@
+-- NOOP

server/src/main/resources/org/cloudfoundry/identity/uaa/db/mysql/V4_112__Users_id_to_Varchar.sql

@@ -0,0 +1,2 @@
+ALTER TABLE users MODIFY id VARCHAR(36) NOT NULL;
+ALTER TABLE oauth_client_details MODIFY created_by VARCHAR(36) NULL;

server/src/main/resources/org/cloudfoundry/identity/uaa/db/postgresql/V4_112__Users_id_to_Varchar_MySQL.sql

@@ -0,0 +1 @@
+-- NOOP

server/src/test/java/org/cloudfoundry/identity/uaa/annotations/WithDatabaseContext.java

@@ -5,7 +5,6 @@
 import org.cloudfoundry.identity.uaa.util.beans.PasswordEncoderConfig;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.springframework.context.annotation.ImportResource;
-import org.springframework.test.context.ActiveProfiles;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit.jupiter.SpringExtension;
 import org.springframework.test.context.web.WebAppConfiguration;
@@ -19,7 +18,6 @@
 @Retention(RetentionPolicy.RUNTIME)
 @ExtendWith(SpringExtension.class)
 @ExtendWith(PollutionPreventionExtension.class)
-@ActiveProfiles("default")
 @WebAppConfiguration
 @ContextConfiguration(classes = {
         DatabaseOnlyConfiguration.class,