Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Development branch for v3.0.0 #264

Draft
wants to merge 66 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
66 commits
Select commit Hold shift + click to select a range
efb4bf5
speedup: use fast, async cache
Pfeil Aug 28, 2024
160bfe0
speedup: use default work stealing executor for "async" cache
Pfeil Aug 29, 2024
f976bdd
speedup: use extra executors for validation and deserialization
Pfeil Aug 29, 2024
0d8ce40
Merge branch 'master' into validation-speedup-experiments
Pfeil Nov 8, 2024
c7e6cbb
chore: rename TypeRegistry to DtrTest, as it depends on dtr-test sche…
Pfeil Nov 16, 2024
439e8b0
feat: add main code base for type-api support
Pfeil Nov 20, 2024
5ee3e4a
feat: more flexible validation
Pfeil Nov 20, 2024
dabaa4d
feat: use virtual threads for async execution
Pfeil Nov 20, 2024
3ff56e4
cleanup: remove unused methods which depended on the old TypeDefiniti…
Pfeil Nov 20, 2024
91538cb
fix: assumed wrong type of baseUrl
Pfeil Nov 20, 2024
df87d27
chore: cleanup, formatting, typos
Pfeil Nov 20, 2024
a04dc32
feat: introduce SchemaSetGenerator to support different schema genera…
Pfeil Dec 12, 2024
9b89c01
Merge remote-tracking branch 'origin/master' into validation-speedup-…
Pfeil Dec 12, 2024
0695cc6
feat: read additionalAttributes allowed from profiles and consider th…
Pfeil Jan 3, 2025
8544909
fix: throw error on PID not found
Pfeil Jan 7, 2025
7938072
fix: missing slash after type api endpoint
Pfeil Jan 7, 2025
85db2a0
fix(tests): check assumptions in etag test setup
Pfeil Jan 7, 2025
19b030a
Merge remote-tracking branch 'origin/master' into validation-speedup-…
Pfeil Jan 7, 2025
e7361f4
fix(tests): type api test for complex type now properly reflects the …
Pfeil Jan 7, 2025
18a06ba
test: some general profile attributes with type API
Pfeil Jan 7, 2025
69b0d57
cleanup: remove unused domain classes
Pfeil Jan 8, 2025
aaba94d
fix: apply json schema validation correctly
Pfeil Jan 8, 2025
c4d1aa2
fix: a attribute which is not registered, shall being presented as a …
Pfeil Jan 8, 2025
24c5beb
cleanup: apply linter suggestions
Pfeil Jan 8, 2025
186e4a8
fix(tests): profiles which do not specify their handling of additiona…
Pfeil Jan 8, 2025
e61ba54
fix(tests): sanity check about the number of tests to execute with lo…
Pfeil Jan 8, 2025
4fd4543
cleanup: apply linter suggestions
Pfeil Jan 8, 2025
04bff17
docs: add note about why this test currently fails
Pfeil Jan 8, 2025
ef0242e
fix: use different executor per cache
Pfeil Jan 13, 2025
7fdf554
refactor: use modern http client in dtr test schema generator
Pfeil Jan 13, 2025
dc7fa02
fix(CI): use temurin openJDK instead of zulu openJDK.
Pfeil Jan 13, 2025
bc2bb02
fix: throw correct errors on pid creation
Pfeil Jan 13, 2025
d1144f3
cleanup: spelling and outdated comments
Pfeil Jan 13, 2025
70ee315
cleanup: avoid log spamming in the large record tests.
Pfeil Jan 13, 2025
3ea8931
fix(test): improve output and adjust test to new profile behavior
Pfeil Jan 14, 2025
b12a4dd
fix(test): fix invalid values before validation of real record
Pfeil Jan 14, 2025
8a9609b
cleanup: remove useless newline in exception message
Pfeil Jan 14, 2025
90539c2
cleanup: make executor types easily changeable in one line
Pfeil Jan 14, 2025
643e2be
cleanup: rewrite unpacking of exceptions without recursion
Pfeil Jan 15, 2025
d2b5582
cleanup: better names for some config properties
Pfeil Jan 17, 2025
a19e2ea
cleanup: use clearer name for additional attributes
Pfeil Jan 17, 2025
d9d507c
docs: document config properties in application-default.properties
Pfeil Jan 17, 2025
698392e
Merge remote-tracking branch 'origin/master' into validation-speedup-…
Pfeil Jan 17, 2025
c1fdc5d
feat: warn via logging if the cache size is unusual low.
Pfeil Jan 21, 2025
0aada09
feat: implement dockerized benchmarks
Pfeil Jan 21, 2025
c6fb821
fix: make sure test script does not influence caches or else in prepa…
Pfeil Jan 22, 2025
4c9350b
cleanup: remove TODO comment
Pfeil Jan 22, 2025
a3c496b
benchmarks: wait longer between health check and starting the requests
Pfeil Jan 22, 2025
8fe47fc
benchmarks: avoid virtual threads for now until the ecosystem is more…
Pfeil Jan 22, 2025
ad429ba
benchmarks: add first results
Pfeil Jan 22, 2025
e4aa487
CI: test zulu now that we do not use virtual threads anymore
Pfeil Jan 22, 2025
81f9d23
CI: remove zulu
Pfeil Jan 22, 2025
85e1ddf
docs: fix information about validation times
Pfeil Jan 22, 2025
a92bd4d
feat: log warnings if a request took longer than 400ms
Pfeil Jan 23, 2025
0927d2b
test: test for records with invalid values
Pfeil Jan 24, 2025
a29e029
cleanup: fix linter warnings
Pfeil Jan 24, 2025
471a5a3
Merge pull request #218 from kit-data-manager/validation-speedup-expe…
Pfeil Jan 24, 2025
76e4641
version: set version to v3.0.0-rc1
Pfeil Jan 24, 2025
8c2377b
Merge branch 'master' into dev-v3
Pfeil Jan 27, 2025
4c17b00
fix: use a EOSC DTR type to avoid regular issues with dtr-test within…
Pfeil Jan 27, 2025
61c449f
Implement meta-resolver.
Pfeil Jan 29, 2025
2e0b6db
chore: more ergonomic implementation of index calculation in HandleIndex
Pfeil Jan 29, 2025
58f9984
chore: optimize record-to-handleValues conversion and explicitly unco…
Pfeil Jan 29, 2025
919c4de
chore: move HandleProtocolAdapter to its own module
Pfeil Jan 29, 2025
76236f5
chore: split HandleProtocolAdapter into different responsibilities.
Pfeil Jan 29, 2025
3608964
Merge pull request #271 from kit-data-manager/separate-resolver
Pfeil Jan 29, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.3")
implementation("edu.kit.datamanager:repo-core:1.2.5")
// 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