Skip to content

Commit

Permalink
Merge pull request #218 from kit-data-manager/validation-speedup-expe…
Browse files Browse the repository at this point in the history
…riments

Type-Api support and validation speedup
  • Loading branch information
Pfeil authored Jan 24, 2025
2 parents 459f0c0 + a29e029 commit 471a5a3
Show file tree
Hide file tree
Showing 60 changed files with 1,618 additions and 1,626 deletions.
10 changes: 6 additions & 4 deletions .github/workflows/gradle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,16 @@ jobs:
strategy:
matrix:
operating-system: [ubuntu-latest, macOS-latest, windows-latest]
jdk: [ 21 ] # (open)JDK releases
jdk: [ 21 ]
distro: ['temurin']

steps:
- uses: actions/checkout@v4
- name: Set up openJDK version
uses: actions/setup-java@v4
with:
java-version: ${{ matrix.jdk }}
distribution: 'zulu' # =openJDK
distribution: ${{ matrix.distro }}
- name: Setup Gradle
uses: gradle/actions/setup-gradle@v4
- name: Build and Test with Gradle
Expand All @@ -45,7 +46,8 @@ jobs:
- name: Docker build and test
if: matrix.operating-system == 'ubuntu-latest' && matrix.jdk == 21
run: |
curl --location --remote-name https://github.com/Orange-OpenSource/hurl/releases/download/4.0.0/hurl_4.0.0_amd64.deb
sudo dpkg -i hurl_4.0.0_amd64.deb
VERSION=6.0.0
curl --location --remote-name https://github.com/Orange-OpenSource/hurl/releases/download/$VERSION/hurl_${VERSION}_amd64.deb
sudo apt update && sudo apt install ./hurl_${VERSION}_amd64.deb
time bash ./docker/test_docker.sh
shell: bash
26 changes: 23 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,27 @@ PID = prefix + (branding + uniquely-generated-string)

All other configuration properties affect only the `uniquely-generated-string`. For example, you may choose a different generation method (UUID (default) or Hex Chunks) enforce casing (lower-case, upper-case).

## What PID record validation means

Our validation steps include the following:

1. For each attribute, check if the values are valid according to their type specification.
- Values are considered to be in JSON format. If a type is complex, this implies a string-serialized JSON object.
2. For each attribute which indicates a profile, resolve the profile definitions from the values of the attributes.
- Attributes which indicates profiles are internally known, but may be added using the [configuration](https://github.com/kit-data-manager/pit-service/blob/master/config/application-default.properties).
3. For each profile definition, check if mandatory attributes are present.
4. For each profile definition, check if only repeatable attributes have multiple occurrences.

This implies the following properties:

- A profile is not required,
- but all profiles which are present are being used for validation,
- and all of them have to pass.
- Additional attributes are allowed if specified in the configuration of the Typed PID Maker instance.
- Otherwise, they are not allowed. Which makes it almost impossible to use multiple profiles.
- Only dtr-test supports an "additionalAttributesAllowed" boolean property per profile,
- But as it will not last and other DTRs do currently not support it, we don't support it either.

## How to build

> Note: Alternatively, you can use the docker image.
Expand All @@ -111,9 +132,8 @@ All other configuration properties affect only the `uniquely-generated-string`.
- Building (with tests): `./gradlew clean build`
- Building (with verbose test output) `./gradlew -Dprofile=verbose clean build`
- Building (without tests): `./gradlew clean build -x test`
- Run docker integration tests:
- `./gradlew clean build` (by default, this will reuse the local build)
- `time bash ./docker/test_docker.sh` (runs test script)
- Run docker integration tests: `time bash ./docker/test_docker.sh` (will reuse the local build)
- Run dockerized validation benchmarks: `time bash ./docker/test_docker.sh benchmark` (will reuse the local build)
- Doing a release: `./gradlew clean build release`
- Will prompt you about version number to use and next version number
- Will make a git tag which can later be used in a GitHub release
Expand Down
4 changes: 2 additions & 2 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ dependencies {
// dependencies. It will automatically choose the fitting ones.
implementation("edu.kit.datamanager:service-base:1.3.2")
implementation("edu.kit.datamanager:repo-core:1.2.3")
// com.google.common, LoadingCache
implementation("com.google.guava:guava:33.4.0-jre")
// AsyncLoadingCache https://github.com/ben-manes/caffeine
implementation("com.github.ben-manes.caffeine:caffeine:3.1.8")

// spring core, e.g. @EnableJpaRepositories
implementation "org.springframework:spring-core"
Expand Down
38 changes: 31 additions & 7 deletions config/application-default.properties
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ pit.pidsystem.implementation = LOCAL
#pit.pidsystem.handle-protocol.credentials.handleIdentifierPrefix = 21.T11981
#pit.pidsystem.handle-protocol.credentials.userHandle = 21.T11981/USER01
#pit.pidsystem.handle-protocol.credentials.privateKeyPath = test_prefix_data/21.T11981_USER01_300_privkey.bin

# The handle system supports the redirection of web browsers to a URL.
# If your records may have such a URL stored in an attribute, you can
# list the attributes here. The first attribute to be found will have
Expand All @@ -169,17 +170,40 @@ pit.pidsystem.implementation = LOCAL
pit.pidsystem.handle-protocol.handleRedirectAttributes = {'21.T11148/b8457812905b83046284'}

### Base URL for the DTR used. ###
# Currently, we support the DTRs of GWDG/ePIC. Currently known instances:
# - http://dtr-test.pidconsortium.eu/, https://dtr-test.pidconsortium.net/
# - http://dtr-pit.pidconsortium.eu/, http://dtr-pit.pidconsortium.net/
# - http://typeregistry.org/
pit.typeregistry.baseURI = https://dtr-test.pidconsortium.eu/
# Currently, we support the DTRs of GWDG/ePIC.
pit.typeregistry.baseURI = https://typeapi.lab.pidconsortium.net
# If the attribute(s) keys/types in your PID records are not being
# recognized as such, please contact us.
# As a workaround, add them to this list.
# pit.validation.profileKeys = {}

### As this service is a RESTful serice without GUI, CSRF protection is not required. ###
pit.security.enable-csrf: false
### You may define patterns here for services which are allowed for communication. (CORS) ###
pit.security.allowedOriginPattern: http*://localhost:[*]

### Caching settings for validation ###
# The maximum number of entries in the cache.
# pit.typeregistry.cache.maxEntries:1000
#
# The time in minutes after which Entries will expire, starting from the
# last update.
# pit.typeregistry.cache.lifetimeMinutes:10

# Profiles may disallow additional attributes in the PID records. This
# option may be used to override this behavior for this instance.
# If set to false, it will behave as the profiles describe.
# If set to true, additional attributes will always be allowed.
pit.validation.alwaysAllowAdditionalAttributes = true

### DANGEROUS OPTIONS! Please read carefully! ########################################
# This will disable validation. It is only meant for testing and rare cases
# where a DTR may not be available or an external validator is being
# used.
#
# pit.validation.strategy = debug-none
### DANGEROUS OPTIONS! Please read carefully! ########################################

#######################################################
#################### PID GENERATOR ####################
#######################################################
Expand Down Expand Up @@ -207,7 +231,7 @@ pit.pidgeneration.casing = lower
# Default: 4
# pit.pidgeneration.num-chunks = 4

### DANGEROUS OPTION! Please read carefully! ########################################
### DANGEROUS OPTIONS! Please read carefully! ########################################
# Please keep this option as a last resort vor special use-cases
# where you need total control about the PID suffix you want to create.
# In addition to authentication, we recommend fully hide the Typed PID Maker behind
Expand All @@ -218,7 +242,7 @@ pit.pidgeneration.casing = lower
# => PID = "abc/def" (delimiter may depend on PID system)
#
# pit.pidgeneration.custom-client-pids-enabled = false
### DANGEROUS OPTION! Please read carefully! ########################################
### DANGEROUS OPTIONS! Please read carefully! ########################################

################################
######## Database ##############
Expand Down
11 changes: 6 additions & 5 deletions config/application-docker.properties
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ pit.pidsystem.implementation = LOCAL
#pit.pidsystem.handle-protocol.credentials.handleIdentifierPrefix = 21.T11981
#pit.pidsystem.handle-protocol.credentials.userHandle = 21.T11981/USER01
#pit.pidsystem.handle-protocol.credentials.privateKeyPath = test_prefix_data/21.T11981_USER01_300_privkey.bin

# The handle system supports the redirection of web browsers to a URL.
# If your records may have such a URL stored in an attribute, you can
# list the attributes here. The first attribute to be found will have
Expand All @@ -169,11 +170,11 @@ pit.pidsystem.implementation = LOCAL
pit.pidsystem.handle-protocol.handleRedirectAttributes = {'21.T11148/b8457812905b83046284'}

### Base URL for the DTR used. ###
# Currently, we support the DTRs of GWDG/ePIC. Currently known instances:
# - http://dtr-test.pidconsortium.eu/, https://dtr-test.pidconsortium.net/
# - http://dtr-pit.pidconsortium.eu/, http://dtr-pit.pidconsortium.net/
# - http://typeregistry.org/
pit.typeregistry.baseURI = https://dtr-test.pidconsortium.eu/
# Currently, we support the DTRs of GWDG/ePIC.
pit.typeregistry.baseURI = https://typeapi.lab.pidconsortium.net
# If the attribute(s) keys/types in your PID records are not being recognized as such, please contact us.
# As a workaround, add them to this list:
pit.validation.profileKeys = {}

### As this service is a RESTful serice without GUI, CSRF protection is not required. ###
pit.security.enable-csrf: false
Expand Down
42 changes: 31 additions & 11 deletions docker/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,45 @@

There are two images:

1. `Dockerfile-build-in-image` will build the image in the container. The result is still a clean image as it uses multi-stage-builds to only include what it really needs to run. This is good for a container release.
2. `Dockerfile-reuse-local-build` will reuse the build on your local machine. This is good for local development and reusing CI artifacts.
- `Dockerfile-build-in-image`
- Will build the image in the container. The result is still a clean image as it uses multi-stage-builds to only include what it really needs to run.
- Best for making a container release.
- Used by CI to create an image for publication.
- `Dockerfile-reuse-local-build`
- Will reuse the build on your local machine.
- Good for local development and reusing CI artifacts.
- Used in `test_docker.sh` and build/test CI.

The CI will use the first Dockerfile to build and provide images.

## Testing
## Purpose

The test script `docker_tests.sh` will:
For the recommended usage instructions, see the main readme file. The test script `docker_tests.sh` will:

- build and run a container, named `typed-pid-maker-test`
- without parameters, it will use the `Dockerfile-reuse-local-build` definition.
- given an arbitrary argument (we recommend "release" or "build-in-image" for readability), it will use the `Dockerfile-build-in-image` definition.
- execute all tests in the `tests` subfolder
- if the script gets `benchmark` as its first parameter:
- execute benchmark tests repeatedly
- else (default):
- execute all tests in the `tests` subfolder
- stop and delete the container

The idea behind these tests is to have basic tests from a very practical perspective (integration tests, component/service tests). The goal is to test the docker container, but also the standard configuration (application.properties) and the spring setup in general. Examples:
The idea behind the tests is to have basic tests from a very practical perspective (integration tests, component/service tests). The goal is to test the docker container, but also the standard configuration (application.properties) and the spring setup in general. Examples:

- test the creation of a PID: This makes sure that the request actually reaches the handler function. Such a test should exist for all endpoint with different security configurations.
- test the creation of a PID: This makes sure that the request actually reaches the handler function. Such a test should exist for all endpoints with different security configurations.
- test if swagger page is reachable: This makes sure that the spring and openapi/swagger libraries work well together in the current version.
- test if openAPI definition is reachable: same as above.

The goal is **not** to achieve full test coverage here. For this, we have unit tests and integration tests in `src/test`.
The goal is **not** to achieve full test coverage here. For this, we have unit tests and integration tests in `src/test`.

The benchmark mode is available to see the impact of new developments on the validation time needed.

## Benchmark results

Let's collect some results of the benchmarks:

- Mac Studio with M1 Max, 32GB RAM, Sonoma 14.7.2
- Commit: 8fe47fc6781ed08457cedc09d0c0efbccf7359c7
- Executed files: 1000
- Executed requests: 1001 (6.2/s)
- Succeeded files: 1000 (100.0%)
- Failed files: 0 (0.0%)
- Duration: 160903 ms
81 changes: 56 additions & 25 deletions docker/test_docker.sh
100644 → 100755
Original file line number Diff line number Diff line change
@@ -1,46 +1,77 @@
#!/usr/bin/env bash

# hurl is required
if ! command -v hurl &>/dev/null; then
echo "> hurl is required but it's not installed. Aborting." >&2
exit 1
fi

if [ "$1" == "benchmark" ]; then
benchmark_mode=0 # 0 (true) for benchmarks
echo "> benchmark mode"
else
benchmark_mode=1 # 1 (false) for tests
echo "> test mode"
fi

# docker parameters
tag=typed-pid-maker-test
container=typid-test

# meta information for this script
this=${BASH_SOURCE[0]}
echo "this script is at $this"
echo "> this script is at $this"
docker_dir=$(dirname "$this")

echo "build docker image"
echo "> trigger local build"
"$docker_dir"/../gradlew build -x test || exit 1

echo "> build docker image, reusing local build"
sleep .2
# use "standalone" or "release" to build in the docker container
if [ "$1" ]
then
echo " > compiling in container: "
docker build --file $docker_dir/Dockerfile-build-in-image --tag $tag $docker_dir/.. || exit 1
else
echo " > reusing local build: "
docker build --file $docker_dir/Dockerfile-reuse-local-build --tag $tag $docker_dir/.. || exit 1
fi
docker build --file "$docker_dir/Dockerfile-reuse-local-build" \
--tag "$tag" "$docker_dir/.." || exit 1

echo -n "run container: "
docker run -p 8090:8090 --detach --name $container $tag
echo -n "> run container: "
docker run \
--env pit.typeregistry.cache.maxEntries=0 \
-p 8090:8090 \
--detach \
--name $container $tag

echo "> Making sure the service is ready to accept requests"
# lets do something which will definitely not influence caches:
hurl --retry=20 --retry-interval=5000 \
--test "$docker_dir"/tests/resolve-nonexisting.hurl || exit 1
sleep 3
echo "> Service is ready. Starting with actual tests."

#####################################
### tests ###########################
### run tests / benchmarks ################
#####################################
hurl --retry=10 --retry-interval=10000 \
--test "$docker_dir"/tests/*.hurl
failure=$?
echo "> benchmark mode: $benchmark_mode"
if [ "$benchmark_mode" -eq 0 ]; then
echo "> running benchmark"
hurl --retry=20 --retry-interval=5000 \
--test --jobs 1 --repeat 1000 \
"$docker_dir"/tests/benchmark-create_dryrun.hurl
failure=$?
else
echo "> running tests"
hurl --retry=20 --retry-interval=5000 \
--test "$docker_dir"/tests/*.hurl
failure=$?
fi
#####################################

echo -n "stopping container ... "
echo -n "> stopping container ... "
docker container stop $container
echo -n "removing container ... "
echo -n "> removing container ... "
docker container rm $container

if [ $failure -eq 0 ]
then
echo "ALL TESTS SUCCESSFUL."
exit 0
if [ $failure -eq 0 ]; then
echo "> ALL TESTS SUCCESSFUL."
exit 0
else
echo "TESTS FAILED (see output for details)"
exit 1
echo "> TESTS FAILED (see output for details)"
exit 1
fi
Loading

0 comments on commit 471a5a3

Please sign in to comment.