From 4eb4407ef76cac144e4149e7b2ba461b168d3a3b Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 10 Nov 2025 19:46:49 +0000 Subject: [PATCH 1/3] DB: Updated entity scope to use models dynamic table This was hardcoded since the table was always the same, but in some cases Laravel will auto-alias the table name (for example, when in sub-queries) which will break MySQL 5.7 when the scope attempts to use the table name instead of the alias. Needs testing coverage. For #5877 --- app/Entities/Models/EntityScope.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/Entities/Models/EntityScope.php b/app/Entities/Models/EntityScope.php index deb10c5ec..c77b75a16 100644 --- a/app/Entities/Models/EntityScope.php +++ b/app/Entities/Models/EntityScope.php @@ -15,11 +15,12 @@ class EntityScope implements Scope public function apply(Builder $builder, Model $model): void { $builder = $builder->where('type', '=', $model->getMorphClass()); + $table = $model->getTable(); if ($model instanceof Page) { - $builder->leftJoin('entity_page_data', 'entity_page_data.page_id', '=', 'entities.id'); + $builder->leftJoin('entity_page_data', 'entity_page_data.page_id', '=', "{$table}.id"); } else { - $builder->leftJoin('entity_container_data', function (JoinClause $join) use ($model) { - $join->on('entity_container_data.entity_id', '=', 'entities.id') + $builder->leftJoin('entity_container_data', function (JoinClause $join) use ($model, $table) { + $join->on('entity_container_data.entity_id', '=', "{$table}.id") ->where('entity_container_data.entity_type', '=', $model->getMorphClass()); }); } From befc6457051c914d3b0328d60d2e9e50ee2594ca Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 11 Nov 2025 10:24:56 +0000 Subject: [PATCH 2/3] DB: Added initial DB testing docker-based script --- dev/docker/db-testing/Dockerfile | 30 +++++++++++++++++++ dev/docker/db-testing/run.sh | 50 ++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+) create mode 100644 dev/docker/db-testing/Dockerfile create mode 100755 dev/docker/db-testing/run.sh diff --git a/dev/docker/db-testing/Dockerfile b/dev/docker/db-testing/Dockerfile new file mode 100644 index 000000000..69b07a2b5 --- /dev/null +++ b/dev/docker/db-testing/Dockerfile @@ -0,0 +1,30 @@ +FROM ubuntu:24.04 + +# Install additional dependencies +RUN apt-get update && \ + apt-get install -y \ + git \ + wget \ + zip \ + unzip \ + php \ + php-bcmath php-curl php-mbstring php-gd php-xml php-zip php-mysql php-ldap \ + && \ + rm -rf /var/lib/apt/lists/* + +# Take branch as an argument so we can choose which BookStack +# branch to test against +ARG BRANCH=development + +# Download BookStack & install PHP deps +RUN mkdir -p /var/www && \ + git clone https://github.com/bookstackapp/bookstack.git --branch "$BRANCH" --single-branch /var/www/bookstack && \ + cd /var/www/bookstack && \ + wget https://raw.githubusercontent.com/composer/getcomposer.org/f3108f64b4e1c1ce6eb462b159956461592b3e3e/web/installer -O - -q | php -- --quiet --filename=composer && \ + php composer install + +# Set the BookStack dir as the default working dir +WORKDIR /var/www/bookstack + +# Set the default action as running php +ENTRYPOINT ["/bin/php"] diff --git a/dev/docker/db-testing/run.sh b/dev/docker/db-testing/run.sh new file mode 100755 index 000000000..f9944f8e4 --- /dev/null +++ b/dev/docker/db-testing/run.sh @@ -0,0 +1,50 @@ +#!/bin/bash + +BRANCH=${1:-development} + +# Build the container with a known name +docker build --build-arg BRANCH="$BRANCH" -t bookstack:db-testing . + +# List of database containers to test against +containers=( + mysql:5.7 + mysql:8.0 + mysql:8.4 + mysql:9.5 + mariadb:10.2 + mariadb:10.6 + mariadb:10.11 + mariadb:11.4 + mariadb:11.8 + mariadb:12.0 +) + +# Pre-clean-up from prior runs +docker stop bs-dbtest-db +docker network rm bs-dbtest-net + +# Cycle over each database image +for img in "${containers[@]}"; do + echo "Starting tests with $img..." + docker network create bs-dbtest-net + docker run -d --rm --name "bs-dbtest-db" \ + -e MYSQL_DATABASE=bookstack-test -e MYSQL_USER=bookstack -e MYSQL_PASSWORD=bookstack -e MYSQL_ROOT_PASSWORD=password \ + --network=bs-dbtest-net \ + "$img" + sleep 10 + APP_RUN='docker run -it --rm --network=bs-dbtest-net -e TEST_DATABASE_URL="mysql://bookstack:bookstack@bs-dbtest-db:3306" bookstack:db-testing' + $APP_RUN artisan migrate --force --database=mysql_testing + $APP_RUN artisan db:seed --force --class=DummyContentSeeder --database=mysql_testing + $APP_RUN vendor/bin/phpunit + if [ $? -eq 0 ]; then + echo "$img - Success" + else + echo "$img - Error" + read -p "Stop script? [y/N] " ans + [[ $ans == [yY] ]] && exit 0 + fi + + docker stop "bs-dbtest-db" + docker network rm bs-dbtest-net +done + From 8ab9252f9bd1626b87df044b14ce3469b1ca07bf Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 11 Nov 2025 11:23:16 +0000 Subject: [PATCH 3/3] DB: Added extra query tests, updated db-testing scripts Also added skipping to avif tests for environments where GD does not have built-in AVIF support --- app/Entities/Models/Book.php | 3 +-- dev/docker/db-testing/run.sh | 33 ++++++++++++++---------- tests/Entity/EntityQueryTest.php | 44 ++++++++++++++++++++++++++++++++ tests/Uploads/ImageTest.php | 4 +++ 4 files changed, 69 insertions(+), 15 deletions(-) create mode 100644 tests/Entity/EntityQueryTest.php diff --git a/app/Entities/Models/Book.php b/app/Entities/Models/Book.php index afd50797b..1909dbd56 100644 --- a/app/Entities/Models/Book.php +++ b/app/Entities/Models/Book.php @@ -67,8 +67,7 @@ class Book extends Entity implements HasDescriptionInterface, HasCoverInterface, */ public function chapters(): HasMany { - return $this->hasMany(Chapter::class) - ->where('type', '=', 'chapter'); + return $this->hasMany(Chapter::class); } /** diff --git a/dev/docker/db-testing/run.sh b/dev/docker/db-testing/run.sh index f9944f8e4..9898e52ec 100755 --- a/dev/docker/db-testing/run.sh +++ b/dev/docker/db-testing/run.sh @@ -4,19 +4,23 @@ BRANCH=${1:-development} # Build the container with a known name docker build --build-arg BRANCH="$BRANCH" -t bookstack:db-testing . +if [ $? -eq 1 ]; then + echo "Failed to build app container for testing" + exit 1 +fi # List of database containers to test against containers=( - mysql:5.7 - mysql:8.0 - mysql:8.4 - mysql:9.5 - mariadb:10.2 - mariadb:10.6 - mariadb:10.11 - mariadb:11.4 - mariadb:11.8 - mariadb:12.0 + "mysql:5.7" + "mysql:8.0" + "mysql:8.4" + "mysql:9.5" + "mariadb:10.2" + "mariadb:10.6" + "mariadb:10.11" + "mariadb:11.4" + "mariadb:11.8" + "mariadb:12.0" ) # Pre-clean-up from prior runs @@ -28,10 +32,13 @@ for img in "${containers[@]}"; do echo "Starting tests with $img..." docker network create bs-dbtest-net docker run -d --rm --name "bs-dbtest-db" \ - -e MYSQL_DATABASE=bookstack-test -e MYSQL_USER=bookstack -e MYSQL_PASSWORD=bookstack -e MYSQL_ROOT_PASSWORD=password \ + -e MYSQL_DATABASE=bookstack-test \ + -e MYSQL_USER=bookstack \ + -e MYSQL_PASSWORD=bookstack \ + -e MYSQL_ROOT_PASSWORD=password \ --network=bs-dbtest-net \ - "$img" - sleep 10 + "$img" + sleep 20 APP_RUN='docker run -it --rm --network=bs-dbtest-net -e TEST_DATABASE_URL="mysql://bookstack:bookstack@bs-dbtest-db:3306" bookstack:db-testing' $APP_RUN artisan migrate --force --database=mysql_testing $APP_RUN artisan db:seed --force --class=DummyContentSeeder --database=mysql_testing diff --git a/tests/Entity/EntityQueryTest.php b/tests/Entity/EntityQueryTest.php new file mode 100644 index 000000000..7d3fd38ce --- /dev/null +++ b/tests/Entity/EntityQueryTest.php @@ -0,0 +1,44 @@ +assertEquals($expected, $query->toSql()); + $this->assertEquals(['book', 'book'], $query->getBindings()); + } + + public function test_joins_in_sub_queries_use_alias_names() + { + $query = Book::query()->whereHas('chapters', function (Builder $query) { + $query->where('name', '=', 'a'); + }); + + // Probably from type limits on relation where not needed? + $expected = 'select * from `entities` left join `entity_container_data` on `entity_container_data`.`entity_id` = `entities`.`id` and `entity_container_data`.`entity_type` = ? where exists (select * from `entities` as `laravel_reserved_%d` left join `entity_container_data` on `entity_container_data`.`entity_id` = `laravel_reserved_%d`.`id` and `entity_container_data`.`entity_type` = ? where `entities`.`id` = `laravel_reserved_%d`.`book_id` and `name` = ? and `type` = ? and `laravel_reserved_%d`.`deleted_at` is null) and `type` = ? and `entities`.`deleted_at` is null'; + $this->assertStringMatchesFormat($expected, $query->toSql()); + $this->assertEquals(['book', 'chapter', 'a', 'chapter', 'book'], $query->getBindings()); + } + + public function test_book_chapter_relation_applies_type_condition() + { + $book = $this->entities->book(); + $query = $book->chapters(); + $expected = 'select * from `entities` left join `entity_container_data` on `entity_container_data`.`entity_id` = `entities`.`id` and `entity_container_data`.`entity_type` = ? where `entities`.`book_id` = ? and `entities`.`book_id` is not null and `type` = ? and `entities`.`deleted_at` is null'; + $this->assertEquals($expected, $query->toSql()); + $this->assertEquals(['chapter', $book->id, 'chapter'], $query->getBindings()); + + $query = Book::query()->whereHas('chapters'); + $expected = 'select * from `entities` left join `entity_container_data` on `entity_container_data`.`entity_id` = `entities`.`id` and `entity_container_data`.`entity_type` = ? where exists (select * from `entities` as `laravel_reserved_%d` left join `entity_container_data` on `entity_container_data`.`entity_id` = `laravel_reserved_%d`.`id` and `entity_container_data`.`entity_type` = ? where `entities`.`id` = `laravel_reserved_%d`.`book_id` and `type` = ? and `laravel_reserved_%d`.`deleted_at` is null) and `type` = ? and `entities`.`deleted_at` is null'; + $this->assertStringMatchesFormat($expected, $query->toSql()); + $this->assertEquals(['book', 'chapter', 'chapter', 'book'], $query->getBindings()); + } +} diff --git a/tests/Uploads/ImageTest.php b/tests/Uploads/ImageTest.php index a2f03df34..c5a5eb2ba 100644 --- a/tests/Uploads/ImageTest.php +++ b/tests/Uploads/ImageTest.php @@ -73,6 +73,10 @@ class ImageTest extends TestCase public function test_image_display_thumbnail_generation_for_animated_avif_images_uses_original_file() { + if (! function_exists('imageavif')) { + $this->markTestSkipped('imageavif() is not available'); + } + $page = $this->entities->page(); $admin = $this->users->admin(); $this->actingAs($admin);