Fix OAuth2 configuration <--> JSON serialization/deserialization on QT6

[rouault]

No description provided.

[nyalldawson]

canConvert has a different (stupid) meaning on Qt 6. Now it's "is there any way this variant can possibly be turned into a XXX", and the answer is almost always true. It no longer actually checks the value in the variant to do the test! 😱

The qt 6 correct way of doing this is to manually check the userType()

[rouault]

(The above did fix things for me, and test_authmethod_authoauth2method)
I'm not totally sure to understand your suggestion. Do you mean just replace the call to metaType() by userType() ? And removing the test val.canConvert( QMetaType::Int ) ? In this generic code, we don't know what exact enumeration type we have, but we want enumeration types to be converted to integers instead of strings (with numeric value) which was what happened without the fix

[nyalldawson]

yeah I'd suggest just changing val.canConvert( QMetaType::Int ) with val.userType() == QMetaType::Int

[rouault]

yeah I'd suggest just changing val.canConvert( QMetaType::Int ) with val.userType() == QMetaType::Int

that doesn't work. But I've just removed that test: all enumeration types can be converted to int, right ?

[nyalldawson]

I wonder how much work is involved in dropping this dependency. AFAICS it's used in:

• QgsAuthOAuth2Edit/QgsAuthOAuth2Config to parse json strings to a QVariant, which should be trivial to switch to QgsJsonUtils::parseJson
• QgsAuthOAuth2Edit/QgsAuthOAuth2Config to convert a QVariantMap to a json string, which again should be trivial to switch to QgsJsonUtils::jsonFromVariant
• QgsAuthOAuth2Config to set properties on a qobject based on json, and to create json based on qobject properties. These aren't so trivial to swap, but we could either:
  • just copy QJsonWrapper::qvariant2qobject and qobject2qvariant to QgsJsonUtils. There's not a lot in those particular functions.
  • Or, (arguably better), explicitly create a QVariantMap based on the object properties directly and avoid the Qt metaobject introspection entirely. This would be less fragile and easier to understand IMO. (I've been bitten in the past by renaming methods in QgsAuthOAuth2Config and assuming that no compilation failure = everything is fine)

What do you think?

[nyalldawson]

Ping @m-kuhn , who I'm sure would be keen on seeing another embedded library dropped 🤣

[github-actions]

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit 41f5437)

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit 41f5437)

[qgis-bot]

The backport to queued_ltr_backports failed:

The process '/usr/bin/git' failed with exit code 1

stderr

error: could not apply 41f54377e0e... Fix OAuth2 configuration <--> JSON serialization/deserialization on QT6
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config advice.mergeConflict false"

stdout

Auto-merging .ci/test_blocklist_qt6.txt
CONFLICT (content): Merge conflict in .ci/test_blocklist_qt6.txt
Auto-merging .docker/docker-qgis-build.sh
CONFLICT (content): Merge conflict in .docker/docker-qgis-build.sh

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-queued_ltr_backports queued_ltr_backports
# Navigate to the new working tree
cd .worktrees/backport-queued_ltr_backports
# Create a new branch
git switch --create backport-59702-to-queued_ltr_backports
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick 41f54377e0e05c01b60963d5c648478ca9812f24
# Push it to GitHub
git push --set-upstream origin backport-59702-to-queued_ltr_backports
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-queued_ltr_backports

Then, create a pull request where the base branch is queued_ltr_backports and the compare/head branch is backport-59702-to-queued_ltr_backports.

[rouault]

queued_ltr_backport in #59730