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

Remove the loop of calendars when only one is needed #33608

Merged
merged 1 commit into from
Oct 3, 2022

Conversation

miaulalala
Copy link
Contributor

Fixes #33599

Copy link
Member

@tcitworld tcitworld left a comment

Choose a reason for hiding this comment

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

Good enough for now, but I suggest the following to make psalm happy properly (possibly as an extra PR):

  • Add a new interface DirectURIAccess providing the getCalendarByUri and getSubscriptionByUri.
  • Implement getSubscriptionByUri in CalDAV backend
  • This allows to do pretty much the same for subscriptions below (more speed ⚡ )
  • Put all of this into a if ($this->caldavBackend instanceof DirectURIAccess) check so that psalm is happy and we're keeping things clean).

@tcitworld
Copy link
Member

Testing trashbin should work the same btw, through a TrashbinSupport interface.

@CarlSchwan
Copy link
Member

Still some failures in drone

@miaulalala miaulalala force-pushed the perf/improve-getCalendarsForUsers branch from e8fff40 to b196f4c Compare September 5, 2022 13:46
@miaulalala miaulalala requested a review from kesselb September 5, 2022 13:47
@miaulalala
Copy link
Contributor Author

Drone failures with file sharing acceptance tests (unrelated?)

@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Sep 5, 2022
@nickvergessen
Copy link
Member

Maybe a rebase can tell?

@st3iny st3iny force-pushed the perf/improve-getCalendarsForUsers branch from b196f4c to 2f0d6f7 Compare September 13, 2022 12:38
@st3iny
Copy link
Member

st3iny commented Sep 13, 2022

Rebased to (hopefully) fix tests.

@ChristophWurst
Copy link
Member

Drone restarted

@miaulalala miaulalala force-pushed the perf/improve-getCalendarsForUsers branch from 2f0d6f7 to 47025d4 Compare September 23, 2022 12:59
@blizzz
Copy link
Member

blizzz commented Oct 1, 2022

Linter is unhappy

@blizzz blizzz added this to the Nextcloud 26 milestone Oct 1, 2022
Signed-off-by: Anna Larch <anna@nextcloud.com>
@miaulalala miaulalala force-pushed the perf/improve-getCalendarsForUsers branch from 47025d4 to 3a8c7b6 Compare October 1, 2022 20:17
@miaulalala
Copy link
Contributor Author

Drone failure due to:



latest: Pulling from nextcloud/continuous-integration-php8.0
--
2 | Digest: sha256:3708f22218d6e3b8477d653fd83bf75230b8b61f0be9c25c162d96a14e89661f
3 | Status: Image is up to date for ghcr.io/nextcloud/continuous-integration-php8.0:latest
4 | + bash tests/drone-run-php-tests.sh \|\| exit 0
5 | =========================
6 | = List of changed files =
7 | =========================
8 | apps/dav/lib/CalDAV/CalendarHome.php
9 | apps/dav/tests/unit/CalDAV/CalendarHomeTest.php
10 | =========================
11 | PHP files are modified
12 | + NOCOVERAGE=true TEST_SELECTION=DB ./autotest.sh mariadb
13 | Using PHP executable /usr/bin/php
14 | Using database oc_autotest
15 | Setup environment for mariadb testing on local storage ...
16 | Waiting for MariaDB initialisation ...
17 | ............................................................................................................................................................................................................................................................................................................
18 | [ERROR] Waited 300 seconds, no response


@blizzz blizzz merged commit f055328 into master Oct 3, 2022
@blizzz blizzz deleted the perf/improve-getCalendarsForUsers branch October 3, 2022 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish performance 🚀
Projects
Development

Successfully merging this pull request may close these issues.

get calendar gets all calendar for user
8 participants