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

QgsPathResolver::readPath/writePath: make it work properly with OGR datasource URI #55983

Closed
wants to merge 1 commit into from

Conversation

rouault
Copy link
Contributor

@rouault rouault commented Jan 24, 2024

(fixes #55975)

@github-actions github-actions bot added this to the 3.36.0 milestone Jan 24, 2024
Copy link

🪟 Windows builds ready!

Windows builds of this PR are available for testing here. Debug symbols for this build are available here.

(Built from commit 351dfa4)

@nyalldawson
Copy link
Collaborator

@rouault isn't this a bug in QgsOgrProviderMetadata::relativeToAbsoluteUri / QgsOgrProviderMetadata::absoluteToRelativeUri instead ?

Those methods don't use QgsOgrProviderMetadata::decodeUri/QgsOgrProviderMetadata::encodeUri, whereas others like QgsMdalProviderMetadata::relativeToAbsoluteUri use encode/decode uri to only pass the file path through the QgsPathResolver methods.

@rouault
Copy link
Contributor Author

rouault commented Jan 24, 2024

isn't this a bug in QgsOgrProviderMetadata::relativeToAbsoluteUri / QgsOgrProviderMetadata::absoluteToRelativeUri

Those methods aren't called by QgsPathResolver::readPath() / writePath(), which is what is used for project loading/saving (cf QgsLayerTreeLayer::writeXml()). They actually implement a workaround for the 'fix' I did in readPath()/writePath()
Probably that a rewrite of readPath()/writePath() using provider metadata relativeToAbsoluteUri() would be cleaner, but I'd be worried of breaking stuff

@nyalldawson
Copy link
Collaborator

Hmm, so shouldn't QgsLayerTreeLayer::writeXml / readXml be adapted to use the correct method? Ie instead of:

    elem.setAttribute( QStringLiteral( "source" ), context.pathResolver().writePath( mRef->publicSource() ) );
    elem.setAttribute( QStringLiteral( "providerKey" ), mRef->dataProvider() ? mRef->dataProvider()->name() : QString() );

it should be something like:

    const QString providerKey = mRef->dataProvider() ? mRef->dataProvider()->name() : QString();
    const QString source = providerKey.isEmpty() ? mRef->publicSource() : QgsProviderRegistry::instance()->relativeToAbsoluteUri( providerKey, mRef->publicSource(), context );
    elem.setAttribute( QStringLiteral( "source" ), source );
    elem.setAttribute( QStringLiteral( "providerKey" ), providerKey  );

(It's weird here that publicSource is used instead of source, but let's not touch that!)

@rouault
Copy link
Contributor Author

rouault commented Jan 24, 2024

Hmm, so shouldn't QgsLayerTreeLayer::writeXml / readXml be adapted to use the correct method? Ie instead of:

Thanks for the suggestion. It seems to solve the issue. Closing that one as superseded per #55988

@rouault rouault closed this Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Trailing slashes are silently removed from filter strings
2 participants