Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
49 changes: 47 additions & 2 deletions lib/CalDAV/Schedule/Plugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -597,8 +597,9 @@ public function getSupportedPrivilegeSet(INode $node, array &$supportedPrivilege
}

/**
* This method looks at an old iCalendar object, a new iCalendar object and
* starts sending scheduling messages based on the changes.
* This method looks at an old iCalendar object, a new iCalendar object and:
* - starts sending scheduling messages based on the changes.
* - ensures the description fields are coherent.
*
* A list of addresses needs to be specified, so the system knows who made
* the update, because the behavior may be different based on if it's an
Expand All @@ -612,6 +613,8 @@ public function getSupportedPrivilegeSet(INode $node, array &$supportedPrivilege
*/
protected function processICalendarChange($oldObject, VCalendar $newObject, array $addresses, array $ignore = [], &$modified = false)
{
$this->ensureDescriptionConsistency($oldObject, $newObject, $modified);

$broker = $this->createITipBroker();
$messages = $broker->parseEvent($newObject, $addresses, $oldObject);

Expand Down Expand Up @@ -1003,4 +1006,46 @@ protected function createITipBroker(): Broker
{
return new Broker();
}

/**
* Ensure the alternate version of the description is removed if only the main one is changed.
*
* @param VCalendar|string|null $oldObject
* @param \Sabre\VObject\Component\VCalendar $vCal
* @param bool $modified a marker to indicate that the original object modified by this process
*/
private function ensureDescriptionConsistency($oldObject, VCalendar $vCal, &$modified)
{
if (!$oldObject) {
return; // No previous version to compare
}

$xAltDescPropName = 'X-ALT-DESC';

// Get presence of description fields
$hasOldDescription = isset($oldObject->VTODO) && isset($oldObject->VTODO->DESCRIPTION);
$hasNewDescription = isset($vCal->VTODO) && isset($vCal->VTODO->DESCRIPTION);
$hasOldXAltDesc = isset($oldObject->VTODO) && isset($oldObject->VTODO->{$xAltDescPropName});
$hasNewXAltDesc = isset($vCal->VTODO) && isset($vCal->VTODO->{$xAltDescPropName});
$hasAllDesc = $hasOldDescription && $hasNewDescription && $hasOldXAltDesc && $hasNewXAltDesc;

// If all description fields are present, then verify consistency
if ($hasAllDesc) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add some unit test(s) that will exercise this new code block?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to run actual tests either directly from a Windows terminal or WSL, but I am getting file_put_contents(/MY_PATH/../tempcol/test.txt): Failed to open stream: No such file or directory

I tried everything I thought. Nothing works.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running composer install from WSL gives:

PHP Fatal error: Uncaught ArgumentCountError: array_merge() does not accept unknown named parameters in /usr/share/php/Composer/DependencyResolver/DefaultPolicy.php:84
Stack trace:
#0 [internal function]: array_merge()
#1 /usr/share/php/Composer/DependencyResolver/DefaultPolicy.php(84): call_user_func_array()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know what version of composer you get in WSL.
To update to the latest composer I do:
sudo composer self-update

I have:

$ composer --version
Composer version 2.8.3 2024-11-17 13:13:04
PHP version 7.4.33 (/usr/bin/php7.4)

I have an actual Ubuntu laptop, no Windows. The CI runs in Ubuntu Linux, no Windows anywhere. Maybe use VirtualBox and create a full Ubuntu VM?

The "Failed to open stream" message is maybe coming from a line in ServerPropsTest.php

        file_put_contents(\Sabre\TestUtil::SABRE_TEMPDIR.'col/test.txt', 'Test contents');

SABRE_TEMPDIR should be a constant __DIR__.'/../temp/' that already has / on the end, but somehow the / must be missing and the code ends up trying to write to a directory called testcol.

Try adding a / in front of col

Copy link
Author

@djon2003 djon2003 Dec 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

....SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS   61 / 1645 (  3%)
SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS............  122 / 1645 (  7%)
.............................................................  183 / 1645 ( 11%)
.............................................................  244 / 1645 ( 14%)
.............................................................  305 / 1645 ( 18%)
.............................................................  366 / 1645 ( 22%)
.............................................................  427 / 1645 ( 25%)
.............................................................  488 / 1645 ( 29%)
.............................................................  549 / 1645 ( 33%)
.............................................................  610 / 1645 ( 37%)
.............................................................  671 / 1645 ( 40%)
.........SSSSSSSSSSSSSSSSSSSSSSSSSSSS........................  732 / 1645 ( 44%)
.............................................................  793 / 1645 ( 48%)
.................................................SSSS........  854 / 1645 ( 51%)
.............................................................  915 / 1645 ( 55%)
.............................................................  976 / 1645 ( 59%)
.......................................SSSSSSSSSSSSSSSS...... 1037 / 1645 ( 63%)
............................................................. 1098 / 1645 ( 66%)
.........................................SSSSSSSSSSSSSSSSSS.. 1159 / 1645 ( 70%)
............................................................. 1220 / 1645 ( 74%)
............................................................. 1281 / 1645 ( 77%)
............................................................. 1342 / 1645 ( 81%)
............................................................. 1403 / 1645 ( 85%)
............................................................. 1464 / 1645 ( 88%)
............................................................. 1525 / 1645 ( 92%)
.....SSSSSSSSSSSSSSSSSSSSSSSSSS.............................. 1586 / 1645 ( 96%)
...........................................................   1645 / 1645 (100%)

Time: 00:08.670, Memory: 54.00 MB

OK, but incomplete, skipped, or risky tests!
Tests: 1645, Assertions: 2909, Skipped: 198.

I created myself a Docker container to run the unit tests. Is this a normal execution result? (I still did not add mine, I will when I will be sure that this is the normal expections). I will add this Dockerfile with my PR so others could use it to test easily.

The docker is setup using Ubuntu 24.04, with your version of PHP and composer.

I have the same result with or without running php -S localhost:8000 -t vendor/sabre/http/tests/www.

BTW, the last part of the path (vendor/sabre/http/tests/www) does not exist when:

  • Cloning repo
  • composer install
  • Now I should run the command in CONTRIBUTION.md, the one above, but surely, it tells me that the folder doesn't exist

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@phil-davis I pushed a commit including docker support for the unit tests.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is the normal unit test result. Some unit tests need exgtra environment set up, so they are skipped by default (the "S").

// Get descriptions
$oldDescription = (string) $oldObject->VTODO->DESCRIPTION;
$newDescription = (string) $vCal->VTODO->DESCRIPTION;
$oldXAltDesc = (string) $oldObject->VTODO->{$xAltDescPropName};
$newXAltDesc = (string) $vCal->VTODO->{$xAltDescPropName};

// Compare descriptions
$isSameDescription = $oldDescription === $newDescription;
$isSameXAltDesc = $oldXAltDesc === $newXAltDesc;

// If the description changed, but not the alternate one, then delete the latest
if (!$isSameDescription && $isSameXAltDesc) {
unset($vCal->VTODO->{$xAltDescPropName});
$modified = true;
}
}
}
}
22 changes: 22 additions & 0 deletions tests/docker/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
FROM ubuntu:24.04

# Install PHP
RUN apt update -y
RUN apt install ca-certificates -y
ADD php.sources /etc/apt/sources.list.d/
RUN apt update -y
RUN apt install -y php7.4 php7.4-xml php7.4-mbstring php7.4-curl php7.4-sqlite3
RUN apt install -y curl zip git

# Install composer
RUN php -r "copy('https://getcomposer.org/installer', 'composer-setup.php');"
RUN php composer-setup.php --version=2.8.3
RUN mv composer.phar /usr/local/bin/composer
RUN rm composer-setup.php

# Setup tests
RUN mkdir /src/
ADD run-tests.sh .
RUN chmod +x run-tests.sh

ENTRYPOINT ["/run-tests.sh"]
28 changes: 28 additions & 0 deletions tests/docker/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
USAGE
=====

- Install Docker

REPO
----

- Download the folder **/tests/docker** from the repository
- Goto that folder using a terminal
- Run `docker compose up`
- Run `docker run --rm -ti sabre-dav-unit-tests`

LOCAL
-----

- Goto **$local_repo_path/tests/docker** using a terminal
- Run `docker compose up`
- Run either:
- `docker run --rm -ti -v $local_repo_path:/test-dir/ sabre-dav-unit-tests`
- `./run-local.sh`


DEV
---

- Apply changes to Dockerfile
- Run `docker compose up --build`
8 changes: 8 additions & 0 deletions tests/docker/docker-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
services:
sabre-dav-unit-tests:
entrypoint: bash -c "echo Execute docker run -ti sabre-dav-unit-tests to start"
container_name: sabre-dav-unit-tests
build:
context: ./
dockerfile: ./Dockerfile
image: sabre-dav-unit-tests:latest
34 changes: 34 additions & 0 deletions tests/docker/php.sources
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
Types: deb
URIs: https://ppa.launchpadcontent.net/ondrej/php/ubuntu/
Suites: noble
Components: main
Signed-By:
-----BEGIN PGP PUBLIC KEY BLOCK-----
.
mQINBGYo0vEBEAC0Semxy5I2b8exRUxJfTKkHR4f5uyS0dTd9vYgMI5T3gsa7ypH
HtE+GiZC+T9m/F9h66+XJMxhuNsKRs7T2In5NSeso9H/ytlSTayUaBtCFfRp6y6b
6ozuRBfqYJGxhjAnIzvNF/Wpp2BvfQm3OrQ7uJJrt5IvzLDC4jPxl/Xs3sTT+Hbk
bkKKprZ3xmy2enuwBaNWR/CUtAz3hbkzL1kGbhX9m3QidFJagVVdDw3aNEwo8ush
djWfF+BajNvpDFYJKBGQbCeagB753Baa5yIN62x+THLnLiKTMDS1e7U0ZDiV9671
noTbtN5TeZeyfsEmeZ8X60x11JIP3yYHYZT70/DyTYX3WC9yQFyIgVOfRlGklMKI
k3TLMmtq8w5Hz1vovwzV7PzaQnmY+uNP2ZbAP4fJ3iFAj0L+u0i1nOFgTy0Lq058
O/FjRrQxuceDDCF+9ThspXMw3Puvz8giuBDCdEda84uC7XWMdqgz/maLfFQjAmyP
Ixi1EMxMlHYyZajpR1cdCfrAIQlnQjHSWmyeCFgXPPfRA71aCcJ7oSrDjogW6Ahd
HRkQRKf1FF9BFzycgSQotfR+7CKfPQh1kghufM9W/spARzA709nGZjXJzgEJLQd3
CDB6dIIxT/0YI36h3Qgfmiiw4twO24MMEqEEPIELz2WJKeWGkdQdcekpxQARAQAB
tB9MYXVuY2hwYWQgUFBBIGZvciBPbmTFmWVqIFN1csO9iQJOBBMBCgA4FiEEuNx+
U5RmVu+85MHdcdrqq0rUyrYFAmYo0vECGwMFCwkIBwIGFQoJCAsCBBYCAwECHgEC
F4AACgkQcdrqq0rUyrYOPQ/+IArA4s1J3op/w7cXek0ieFHWHFDrxPYS+78/LF/J
LoYZw0nIU5Ovr+LzehFMIQU6esgPXwbeCVgwLwat57augAkAYWT0UzH5dE6RKAGr
C2vsHWVfPhQn6UndfzwXc0mTLGQni25aQaZ6k60Dbm/vblejrTQrtAUWoMO3Z1cr
NDGJ3Z9DCxtr2o9gRYUI6HwLHJtobTIeI5xsr5x+GvXiIAVCPa3ZEuRL6jMQfqfS
C43mpuiS1kGgsnQLs2DbN7EFCfiJoNX1QzZu25zg+IS9PXbCJnheZWnH0rwUSb/N
hZPcSefGlNlhr824OfT30v79hQnw59XbsfV270O9jPbD4kttN+OiszbU66zsuiOh
BO46XCckQPqDkBMw56GPFuVrQgGb1thXvn67URJgPyJhwauBWKPNAJ9Ojuo+yVq/
hdR1VNWThXQbZgaGSWrbjt6FdYtQb9VX88uu5gFDmr180HogHNUDUcqNLLdnjfFs
4DyJlusQ5I/a7cQ7nlkNgxAmHszwO/mGLBuGljDUYkwZDW9nqP1Q5Q2jMtrhgXvR
2SOtufvecUbB7+eoRSaOnu7CNMATG6LocFEMzhKUde1uZTfWSqnYEcdqoFJMi46y
qaNxhiNLsQ5OBMbgSp2zCbQxRBdITMVvBR5YjCetUIGEs6T1yQ5wh5Xpoi34ShHn
v38=
=kFlZ
-----END PGP PUBLIC KEY BLOCK-----
3 changes: 3 additions & 0 deletions tests/docker/run-local.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#!/bin/bash

docker run -ti --rm -v $(realpath "$(dirname $0)/../../"):/test-dir/ sabre-dav-unit-tests
57 changes: 57 additions & 0 deletions tests/docker/run-tests.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
#!/bin/bash

function executeTests()
# $1 = Source directory
{
src="$1"
(
cd "$src"
composer install

mkdir -p vendor/sabre/http/tests/www
php -S localhost:8000 -t vendor/sabre/http/tests/www &

composer phpunit
)
}

function downloadRepo()
{
local repoUrl=
while [ -z "$repoUrl" ]
do
read -p "Please enter public repository URL:" repoUrl

if [ -z "$repoUrl" ]; then echo "Repository URL is mandatory"; fi
done

local branchName=
read -p "Enter the branch name [default: master]" branchName
branchName=${branchName:-master}

echo "Are you sure you want to test with?"
echo "URL: $repoUrl"
echo "Branch: $branchName"
read -p "[y/N]" confirm

if [ "$confirm" != "Y" ] && [ "$confirm" != "y" ]; then
echo "Cancelling test run"
exit
fi

git clone -b "$branchName" --single-branch --depth 1 "$repoUrl" /src/
}

echo "Starting test container"

src=/test-dir/
if [ -d $src ]; then
echo "Using local volume"
git config --global --add safe.directory /test-dir
else
src=/src/
echo "Using repository"
downloadRepo
fi

executeTests "$src"