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

Homesets are not "personal" when they are detected two times (1x personal, 1x non-personal) #847

Open
2 tasks done
rfc2822 opened this issue Jun 11, 2024 · 4 comments · May be fixed by #1222
Open
2 tasks done

Homesets are not "personal" when they are detected two times (1x personal, 1x non-personal) #847

rfc2822 opened this issue Jun 11, 2024 · 4 comments · May be fixed by #1222
Assignees
Labels
bug Something isn't working

Comments

@rfc2822
Copy link
Member

rfc2822 commented Jun 11, 2024

Problem scope

  • I'm sure that this is a DAVx⁵ problem.

App version

  • I'm using the latest available DAVx⁵ version.

Android version and device/firmware type

No response

Steps to reproduce

Create a DAVx5 account with a service that lists home-sets under multiple principals (e.g. mailbox.org). URL hierarchy:

  • the user principal is something like /users/3, which contains both
    • a /carddav / /caldav homeset
    • group memberships in /principals/groups/0 (display name "Addressbooks")
    • and /principals/groups/1(display name "Standardgrouppe").

Actual result

DAVx5 queries the group principals, which again contain the /carddav / /caldav homeset. So the homeset is first detected as personal homeset (directly within the user principal), but then overwritten as non-personal homeset (from the group principals).

Expected result

If a home-set is detected as both personal and non-personal, it should be treated as personal.

Further info

No response

@rfc2822 rfc2822 added the bug Something isn't working label Jun 11, 2024
@rfc2822
Copy link
Member Author

rfc2822 commented Dec 27, 2024

I think it should be possible to consequently query URLs only once.

  1. The personal (as in level=0) collection will always be detected first, and then added to "already queried".
  2. When the homeset is detected again in a higher recursion (level>0), it's already processed and won't be updated with personal=0.

However I'm afraid that it won't be possible "easily". Maybe it's a good time to further refactor the CollectionListRefresher.

@sunkup Can you start with reproducing the issue? And then see how a solution could look like.

@sunkup
Copy link
Member

sunkup commented Jan 2, 2025

I can reproduce and confirm the problem. A very simple solution could be to insert the homeset only if it does not exist already. That works as expected for me.

val existing = homeSetRepository.getByHomesetUrl(service.id, resolvedHomeSetUrl.toString()) != null
if (!existing) {
    logger.fine("Homeset ${service.id}/$resolvedHomeSetUrl does not exist; inserting now")
    homeSetRepository.insertOrUpdateByUrl(
        HomeSet(0, service.id, personal, resolvedHomeSetUrl)
    )
} else
    logger.fine("Homeset ${service.id}/$resolvedHomeSetUrl exists already; not updating")

@rfc2822
Copy link
Member Author

rfc2822 commented Jan 3, 2025

But would that work in the case that:

  1. There's an old homeset (let's say with an old display name) in the database.
  2. On level 0, this homeset is not detected (because it's not a personal one, but of some member group).
  3. Then at level 1 or 2, the homeset is detected. Now it checks whether it's already existing, and it is.
  4. So it won't be updated and the display name stays outdated.

I think the basic approach should work, but maybe existing shouldn't be derived from the database, but from whether this URL has already been fetched during this service detection. This is what I meant with "consequently query URLs only once".

@sunkup
Copy link
Member

sunkup commented Jan 6, 2025

True, when checking against the database we won't update the homesets anymore at all! We would only want to check whether they have been fetched already, as you say.

if (!alreadyFetched.contains(resolvedHomeSetUrl)) {
    homeSetRepository.insertOrUpdateByUrl(
        HomeSet(0, service.id, personal, resolvedHomeSetUrl)
    )
    alreadyFetched += resolvedHomeSetUrl
}

@sunkup sunkup linked a pull request Jan 6, 2025 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Todo
Development

Successfully merging a pull request may close this issue.

2 participants