Skip to content

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

Merged
blizzz merged 1 commit into
masterfrom
perf/improve-getCalendarsForUsers
Oct 3, 2022
Merged

Remove the loop of calendars when only one is needed#33608
blizzz merged 1 commit into
masterfrom
perf/improve-getCalendarsForUsers

Conversation

@miaulalala

Copy link
Copy Markdown
Contributor

Fixes#33599

Comment threadapps/dav/lib/CalDAV/CalendarHome.php Outdated
Comment threadapps/dav/lib/CalDAV/CalendarHome.php Fixed
Comment threadapps/dav/lib/CalDAV/CalendarHome.php Outdated

@tcitworldtcitworld left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
Member

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

@CarlSchwan

Copy link
Copy Markdown
Member

Still some failures in drone

@miaulalala
miaulalalaforce-pushed the perf/improve-getCalendarsForUsers branch from e8fff40 to b196f4cCompareSeptember 5, 2022 13:46
@miaulalala

Copy link
Copy Markdown
ContributorAuthor

Drone failures with file sharing acceptance tests (unrelated?)

@ChristophWurstChristophWurst 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
Copy Markdown
Member

Maybe a rebase can tell?

@st3iny
st3inyforce-pushed the perf/improve-getCalendarsForUsers branch from b196f4c to 2f0d6f7CompareSeptember 13, 2022 12:38
@st3iny

Copy link
Copy Markdown
Member

Rebased to (hopefully) fix tests.

@ChristophWurst

Copy link
Copy Markdown
Member

Drone restarted

@miaulalala
miaulalalaforce-pushed the perf/improve-getCalendarsForUsers branch from 2f0d6f7 to 47025d4CompareSeptember 23, 2022 12:59
@blizzz

Copy link
Copy Markdown
Member

Linter is unhappy

@blizzzblizzz added this to the Nextcloud 26 milestone Oct 1, 2022
Signed-off-by: Anna Larch <anna@nextcloud.com>
@miaulalala
miaulalalaforce-pushed the perf/improve-getCalendarsForUsers branch from 47025d4 to 3a8c7b6CompareOctober 1, 2022 20:17
@miaulalala

Copy link
Copy Markdown
ContributorAuthor

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 masterOct 3, 2022
@blizzz
blizzz deleted the perf/improve-getCalendarsForUsers branch October 3, 2022 08:59
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to releaseReady to be released and/or waiting for tests to finishperformance 🚀

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

get calendar gets all calendar for user

9 participants

@miaulalala@tcitworld@CarlSchwan@nickvergessen@st3iny@ChristophWurst@blizzz@kesselb@github-advanced-security