Merge pull request #60627 from nyalldawson/fix_stac_memory

[stac] Port more API to use unique_ptr
This commit is contained in:
Alexander Bruy 2025-02-18 07:35:01 +00:00 committed by GitHub
commit 427dd0e480
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 134 additions and 126 deletions

View File

@ -127,7 +127,7 @@ void QgsStacController::handleStacObjectReply()
parser.setBaseUrl( reply->url() );
QString error;
QgsStacObject *object = nullptr;
std::unique_ptr< QgsStacObject > object;
switch ( parser.type() )
{
case QgsStacObject::Type::Catalog:
@ -144,7 +144,7 @@ void QgsStacController::handleStacObjectReply()
error = parser.error().isEmpty() ? QStringLiteral( "Parsed STAC data is not a Catalog, Collection or Item" ) : parser.error();
break;
}
mFetchedStacObjects.insert( requestId, object );
mFetchedStacObjects.insert( requestId, object.release() );
emit finishedStacObjectRequest( requestId, error.isEmpty() ? parser.error() : error );
reply->deleteLater();
mReplies.removeOne( reply );
@ -175,8 +175,8 @@ void QgsStacController::handleItemCollectionReply()
parser.setData( data );
parser.setBaseUrl( reply->url() );
QgsStacItemCollection *fc = parser.itemCollection();
mFetchedItemCollections.insert( requestId, fc );
std::unique_ptr<QgsStacItemCollection> fc = parser.itemCollection();
mFetchedItemCollections.insert( requestId, fc.release() );
emit finishedItemCollectionRequest( requestId, parser.error() );
reply->deleteLater();
mReplies.removeOne( reply );
@ -214,11 +214,22 @@ void QgsStacController::handleCollectionsReply()
mReplies.removeOne( reply );
}
std::unique_ptr< QgsStacObject > QgsStacController::takeStacObject( int requestId )
template<class T>
std::unique_ptr<T> QgsStacController::takeStacObject( int requestId )
{
std::unique_ptr< QgsStacObject > obj( mFetchedStacObjects.take( requestId ) );
return obj;
if ( T *downCastObj = dynamic_cast< T * >( obj.get() ) )
{
( void )obj.release();
return std::unique_ptr< T >( downCastObj );
}
return nullptr;
}
template CORE_EXPORT std::unique_ptr< QgsStacItem > QgsStacController::takeStacObject<QgsStacItem>( int requestId );
template CORE_EXPORT std::unique_ptr< QgsStacCatalog > QgsStacController::takeStacObject<QgsStacCatalog>( int requestId );
template CORE_EXPORT std::unique_ptr< QgsStacObject > QgsStacController::takeStacObject<QgsStacObject>( int requestId );
std::unique_ptr< QgsStacItemCollection > QgsStacController::takeItemCollection( int requestId )
{
@ -232,45 +243,6 @@ std::unique_ptr< QgsStacCollections > QgsStacController::takeCollections( int re
return cols;
}
std::unique_ptr< QgsStacObject > QgsStacController::fetchStacObject( const QUrl &url, QString *error )
{
QgsNetworkReplyContent content = fetchBlocking( url );
if ( content.error() != QNetworkReply::NoError )
{
if ( error )
*error = content.errorString();
return nullptr;
}
const QByteArray data = content.content();
QgsStacParser parser;
parser.setData( data );
parser.setBaseUrl( url );
std::unique_ptr< QgsStacObject > object;
switch ( parser.type() )
{
case QgsStacObject::Type::Catalog:
object.reset( parser.catalog() );
break;
case QgsStacObject::Type::Collection:
object.reset( parser.collection() );
break;
case QgsStacObject::Type::Item:
object.reset( parser.item() );
break;
case QgsStacObject::Type::Unknown:
break;
}
if ( error )
*error = parser.error();
return object;
}
std::unique_ptr< QgsStacItemCollection > QgsStacController::fetchItemCollection( const QUrl &url, QString *error )
{
QgsNetworkReplyContent content = fetchBlocking( url );
@ -348,7 +320,7 @@ void QgsStacController::setAuthCfg( const QString &authCfg )
mAuthCfg = authCfg;
}
QgsStacCatalog *QgsStacController::openLocalCatalog( const QString &fileName ) const
std::unique_ptr<QgsStacCatalog> QgsStacController::openLocalCatalog( const QString &fileName ) const
{
QFile file( fileName );
const bool ok = file.open( QIODevice::ReadOnly );
@ -365,7 +337,7 @@ QgsStacCatalog *QgsStacController::openLocalCatalog( const QString &fileName ) c
}
QgsStacCollection *QgsStacController::openLocalCollection( const QString &fileName ) const
std::unique_ptr<QgsStacCollection> QgsStacController::openLocalCollection( const QString &fileName ) const
{
QFile file( fileName );
const bool ok = file.open( QIODevice::ReadOnly );
@ -381,7 +353,7 @@ QgsStacCollection *QgsStacController::openLocalCollection( const QString &fileNa
return parser.collection();
}
QgsStacItem *QgsStacController::openLocalItem( const QString &fileName ) const
std::unique_ptr<QgsStacItem> QgsStacController::openLocalItem( const QString &fileName ) const
{
QFile file( fileName );
const bool ok = file.open( QIODevice::ReadOnly );
@ -396,3 +368,58 @@ QgsStacItem *QgsStacController::openLocalItem( const QString &fileName ) const
parser.setBaseUrl( fileName );
return parser.item();
}
template<class T>
std::unique_ptr<T> QgsStacController::fetchStacObject( const QUrl &url, QString *error )
{
QgsNetworkReplyContent content = fetchBlocking( url );
if ( content.error() != QNetworkReply::NoError )
{
if ( error )
*error = content.errorString();
return nullptr;
}
const QByteArray data = content.content();
QgsStacParser parser;
parser.setData( data );
parser.setBaseUrl( url );
std::unique_ptr< QgsStacObject > object;
switch ( parser.type() )
{
case QgsStacObject::Type::Catalog:
object = parser.catalog();
break;
case QgsStacObject::Type::Collection:
object = parser.collection();
break;
case QgsStacObject::Type::Item:
object = parser.item();
break;
case QgsStacObject::Type::Unknown:
break;
}
std::unique_ptr< T > res;
if ( T *castObject = dynamic_cast< T * >( object.get() ) )
{
( void )object.release();
res.reset( castObject );
}
else
{
QgsDebugError( "Retrieved STAC object could not be cast to expected type" );
}
if ( error )
*error = parser.error();
return res;
}
template CORE_EXPORT std::unique_ptr< QgsStacItem > QgsStacController::fetchStacObject<QgsStacItem>( const QUrl &url, QString *error );
template CORE_EXPORT std::unique_ptr< QgsStacCollection > QgsStacController::fetchStacObject<QgsStacCollection>( const QUrl &url, QString *error );
template CORE_EXPORT std::unique_ptr< QgsStacCatalog > QgsStacController::fetchStacObject<QgsStacCatalog>( const QUrl &url, QString *error );

View File

@ -24,6 +24,7 @@
#include "qgis_core.h"
#include "qgshttpheaders.h"
#include "qgsnetworkreply.h"
#include "qgsstacobject.h"
class QgsStacObject;
class QgsStacCatalog;
@ -56,26 +57,26 @@ class CORE_EXPORT QgsStacController : public QObject
* Returns a STAC Catalog by parsing a local file
* The caller takes ownership of the returned catalog
*/
QgsStacCatalog *openLocalCatalog( const QString &fileName ) const;
std::unique_ptr< QgsStacCatalog > openLocalCatalog( const QString &fileName ) const;
/**
* Returns a STAC Collection by parsing a local file
* The caller takes ownership of the returned collection
*/
QgsStacCollection *openLocalCollection( const QString &fileName ) const;
std::unique_ptr< QgsStacCollection > openLocalCollection( const QString &fileName ) const;
/**
* Returns a STAC Item by parsing a local file
* The caller takes ownership of the returned item
*/
QgsStacItem *openLocalItem( const QString &fileName ) const;
std::unique_ptr< QgsStacItem > openLocalItem( const QString &fileName ) const;
/**
* Fetches a STAC object from \a url using a blocking network request.
* An optional \a error parameter will be populated with any network error information.
* The caller takes ownership of the returned object
*/
std::unique_ptr< QgsStacObject > fetchStacObject( const QUrl &url, QString *error = nullptr );
template<class T> std::unique_ptr< T > fetchStacObject( const QUrl &url, QString *error = nullptr );
/**
* Fetches a feature collection from \a url using a blocking network request.
@ -130,7 +131,7 @@ class CORE_EXPORT QgsStacController : public QObject
* \see fetchStacObjectAsync
* \see finishedStacObjectRequest
*/
std::unique_ptr< QgsStacObject > takeStacObject( int requestId );
template<class T> std::unique_ptr< T > takeStacObject( int requestId );
/**
* Returns the feature collection fetched with the specified \a requestId

View File

@ -69,8 +69,7 @@ QVector<QgsDataItem *> QgsStacItemItem::createChildren()
{
QgsStacController *controller = stacController();
QString error;
std::unique_ptr< QgsStacObject > obj = controller->fetchStacObject( mPath, &error );
setStacItem( obj );
setStacItem( controller->fetchStacObject<QgsStacItem>( mPath, &error ) );
if ( !mStacItem )
return { new QgsErrorItem( this, error, path() + QStringLiteral( "/error" ) ) };
@ -174,16 +173,9 @@ QgsStacController *QgsStacItemItem::stacController()
return nullptr;
}
void QgsStacItemItem::setStacItem( std::unique_ptr< QgsStacObject > &object )
void QgsStacItemItem::setStacItem( std::unique_ptr<QgsStacItem> item )
{
QgsStacItem *item = dynamic_cast<QgsStacItem *>( object.get() );
if ( item )
{
// release object, mStacItem will take ownership of the successfully cast item
( void )object.release();
}
mStacItem.reset( item );
mStacItem = std::move( item );
updateToolTip();
}
@ -195,8 +187,8 @@ QgsStacItem *QgsStacItemItem::stacItem() const
void QgsStacItemItem::itemRequestFinished( int requestId, QString error )
{
QgsStacController *controller = stacController();
std::unique_ptr< QgsStacObject > object = controller->takeStacObject( requestId );
setStacItem( object );
std::unique_ptr< QgsStacItem > object = controller->takeStacObject< QgsStacItem >( requestId );
setStacItem( std::move( object ) );
if ( mStacItem )
{
mIconName = QStringLiteral( "mActionPropertiesWidget.svg" );
@ -322,8 +314,7 @@ QVector<QgsDataItem *> QgsStacCatalogItem::createChildren()
QgsStacController *controller = stacController();
QString error;
std::unique_ptr< QgsStacObject > obj = controller->fetchStacObject( mPath, &error );
setStacCatalog( obj );
setStacCatalog( controller->fetchStacObject< QgsStacCatalog >( mPath, &error ) );
if ( !mStacCatalog )
return { new QgsErrorItem( this, error, path() + QStringLiteral( "/error" ) ) };
@ -462,16 +453,9 @@ void QgsStacCatalogItem::updateToolTip()
}
}
void QgsStacCatalogItem::setStacCatalog( std::unique_ptr< QgsStacObject > &object )
void QgsStacCatalogItem::setStacCatalog( std::unique_ptr<QgsStacCatalog> catalog )
{
QgsStacCatalog *catalog = dynamic_cast<QgsStacCatalog *>( object.get() );
if ( catalog )
{
// release object, mStacCatalog will take ownership of the successfully cast catalog
( void )object.release();
}
mStacCatalog.reset( catalog );
mStacCatalog = std::move( catalog );
if ( mStacCatalog )
{
if ( mName.isEmpty() && !mStacCatalog->title().isEmpty() )
@ -500,12 +484,12 @@ QVector< QgsDataItem * > QgsStacCatalogItem::createItems( const QVector<QgsStacI
if ( !item )
continue;
std::unique_ptr< QgsStacObject > object( item );
std::unique_ptr< QgsStacItem > object( item );
const QString name = item->properties().value( QStringLiteral( "title" ), item->id() ).toString();
QgsStacItemItem *i = new QgsStacItemItem( this, name, item->url() );
i->setStacItem( object );
i->setStacItem( std::move( object ) );
i->setState( Qgis::BrowserItemState::Populated );
contents.append( i );
}
@ -521,12 +505,12 @@ QVector<QgsDataItem *> QgsStacCatalogItem::createCollections( const QVector<QgsS
if ( !col )
continue;
std::unique_ptr< QgsStacObject > object( col );
std::unique_ptr< QgsStacCollection > object( col );
const QString name = col->title().isEmpty() ? col->id() : col->title();
QgsStacCatalogItem *i = new QgsStacCatalogItem( this, name, col->url() );
i->setStacCatalog( object );
i->setStacCatalog( std::move( object ) );
contents.append( i );
}
return contents;

View File

@ -59,7 +59,7 @@ class CORE_EXPORT QgsStacItemItem : public QgsDataItem
QgsStacController *stacController();
//! takes ownership
void setStacItem( std::unique_ptr< QgsStacObject > &object );
void setStacItem( std::unique_ptr< QgsStacItem > item );
//! does not transfer ownership
QgsStacItem *stacItem() const;
@ -87,7 +87,7 @@ class CORE_EXPORT QgsStacCatalogItem : public QgsDataCollectionItem
void updateToolTip();
//! takes ownership
void setStacCatalog( std::unique_ptr< QgsStacObject > &object );
void setStacCatalog( std::unique_ptr< QgsStacCatalog > object );
//! does not transfer ownership
QgsStacCatalog *stacCatalog() const;

View File

@ -83,12 +83,12 @@ QString QgsStacParser::error() const
return mError;
}
QgsStacCatalog *QgsStacParser::catalog()
std::unique_ptr<QgsStacCatalog> QgsStacParser::catalog()
{
return parseCatalog( mData );
}
QgsStacCatalog *QgsStacParser::parseCatalog( const nlohmann::json &data )
std::unique_ptr<QgsStacCatalog> QgsStacParser::parseCatalog( const nlohmann::json &data )
{
try
{
@ -132,7 +132,7 @@ QgsStacCatalog *QgsStacParser::parseCatalog( const nlohmann::json &data )
catalog->setStacExtensions( extensions );
}
return catalog.release();
return catalog;
}
catch ( nlohmann::json::exception &ex )
{
@ -142,12 +142,12 @@ QgsStacCatalog *QgsStacParser::parseCatalog( const nlohmann::json &data )
}
}
QgsStacCollection *QgsStacParser::collection()
std::unique_ptr<QgsStacCollection> QgsStacParser::collection()
{
return parseCollection( mData );
}
QgsStacCollection *QgsStacParser::parseCollection( const nlohmann::json &data )
std::unique_ptr<QgsStacCollection> QgsStacParser::parseCollection( const nlohmann::json &data )
{
try
{
@ -295,7 +295,7 @@ QgsStacCollection *QgsStacParser::parseCollection( const nlohmann::json &data )
collection->setAssets( assets );
}
return collection.release();
return collection;
}
catch ( nlohmann::json::exception &ex )
{
@ -305,12 +305,12 @@ QgsStacCollection *QgsStacParser::parseCollection( const nlohmann::json &data )
}
}
QgsStacItem *QgsStacParser::item()
std::unique_ptr<QgsStacItem> QgsStacParser::item()
{
return parseItem( mData );
}
QgsStacItem *QgsStacParser::parseItem( const nlohmann::json &data )
std::unique_ptr<QgsStacItem> QgsStacParser::parseItem( const nlohmann::json &data )
{
try
{
@ -393,7 +393,7 @@ QgsStacItem *QgsStacParser::parseItem( const nlohmann::json &data )
if ( data.contains( "collection" ) )
item->setCollection( getString( data["collection"] ) );
return item.release();
return item;
}
catch ( nlohmann::json::exception &ex )
{
@ -478,7 +478,7 @@ QString QgsStacParser::getString( const nlohmann::json &data )
return data.is_null() ? QString() : QString::fromStdString( data );
}
QgsStacItemCollection *QgsStacParser::itemCollection()
std::unique_ptr<QgsStacItemCollection> QgsStacParser::itemCollection()
{
std::vector< std::unique_ptr<QgsStacItem> > items;
QVector< QgsStacLink > links;
@ -511,7 +511,7 @@ QgsStacItemCollection *QgsStacParser::itemCollection()
rawItems.append( i.release() );
}
return new QgsStacItemCollection( rawItems, links, numberMatched );
return std::make_unique< QgsStacItemCollection >( rawItems, links, numberMatched );
}
QgsStacCollections *QgsStacParser::collections()

View File

@ -59,28 +59,28 @@ class QgsStacParser
* If parsing failed, NULLPTR is returned
* The caller takes ownership of the returned catalog
*/
QgsStacCatalog *catalog();
std::unique_ptr< QgsStacCatalog > catalog();
/**
* Returns the parsed STAC Collection
* If parsing failed, NULLPTR is returned
* The caller takes ownership of the returned collection
*/
QgsStacCollection *collection();
std::unique_ptr< QgsStacCollection > collection();
/**
* Returns the parsed STAC Item
* If parsing failed, NULLPTR is returned
* The caller takes ownership of the returned item
*/
QgsStacItem *item();
std::unique_ptr< QgsStacItem > item();
/**
* Returns the parsed STAC API Item Collection
* If parsing failed, NULLPTR is returned
* The caller takes ownership of the returned item collection
*/
QgsStacItemCollection *itemCollection();
std::unique_ptr< QgsStacItemCollection > itemCollection();
/**
* Returns the parsed STAC API Collections
@ -96,9 +96,9 @@ class QgsStacParser
QString error() const;
private:
QgsStacItem *parseItem( const nlohmann::json &data );
QgsStacCatalog *parseCatalog( const nlohmann::json &data );
QgsStacCollection *parseCollection( const nlohmann::json &data );
std::unique_ptr< QgsStacItem > parseItem( const nlohmann::json &data );
std::unique_ptr< QgsStacCatalog > parseCatalog( const nlohmann::json &data );
std::unique_ptr< QgsStacCollection > parseCollection( const nlohmann::json &data );
QVector< QgsStacLink > parseLinks( const nlohmann::json &data );
QMap< QString, QgsStacAsset > parseAssets( const nlohmann::json &data );

View File

@ -277,8 +277,7 @@ void QgsStacSourceSelect::cmbConnections_currentTextChanged( const QString &text
void QgsStacSourceSelect::onStacObjectRequestFinished( int requestId, QString error )
{
QgsDebugMsgLevel( QStringLiteral( "Finished object request %1" ).arg( requestId ), 2 );
std::unique_ptr<QgsStacObject> obj( mStac->takeStacObject( requestId ) );
QgsStacCatalog *cat = dynamic_cast<QgsStacCatalog *>( obj.get() );
std::unique_ptr<QgsStacCatalog> cat( mStac->takeStacObject< QgsStacCatalog >( requestId ) );
if ( !cat )
{

View File

@ -86,10 +86,9 @@ void TestQgsStac::testParseLocalCatalog()
{
const QUrl url( QStringLiteral( "file://%1%2" ).arg( mDataDir, QStringLiteral( "catalog.json" ) ) );
QgsStacController c;
std::unique_ptr< QgsStacObject > obj = c.fetchStacObject( url.toString() );
QVERIFY( obj );
QCOMPARE( obj->type(), QgsStacObject::Type::Catalog );
QgsStacCatalog *cat = dynamic_cast<QgsStacCatalog *>( obj.get() );
std::unique_ptr< QgsStacCatalog > cat = c.fetchStacObject< QgsStacCatalog >( url.toString() );
QVERIFY( cat );
QCOMPARE( cat->type(), QgsStacObject::Type::Catalog );
QVERIFY( cat );
QCOMPARE( cat->id(), QLatin1String( "examples" ) );
@ -114,10 +113,9 @@ void TestQgsStac::testParseLocalCollection()
{
const QUrl url( QStringLiteral( "file://%1%2" ).arg( mDataDir, QStringLiteral( "collection.json" ) ) );
QgsStacController c;
std::unique_ptr< QgsStacObject > obj = c.fetchStacObject( url.toString() );
QVERIFY( obj );
QCOMPARE( obj->type(), QgsStacObject::Type::Collection );
QgsStacCollection *col = dynamic_cast<QgsStacCollection *>( obj.get() );
std::unique_ptr< QgsStacCollection > col = c.fetchStacObject< QgsStacCollection >( url.toString() );
QVERIFY( col );
QCOMPARE( col->type(), QgsStacObject::Type::Collection );
QVERIFY( col );
QCOMPARE( col->id(), QLatin1String( "simple-collection" ) );
@ -162,10 +160,9 @@ void TestQgsStac::testParseLocalItem()
{
const QUrl url( QStringLiteral( "file://%1%2" ).arg( mDataDir, QStringLiteral( "core-item.json" ) ) );
QgsStacController c;
std::unique_ptr< QgsStacObject > obj = c.fetchStacObject( url.toString() );
QVERIFY( obj );
QCOMPARE( obj->type(), QgsStacObject::Type::Item );
QgsStacItem *item = dynamic_cast<QgsStacItem *>( obj.get() );
std::unique_ptr< QgsStacItem > item = c.fetchStacObject< QgsStacItem >( url.toString() );
QVERIFY( item );
QCOMPARE( item->type(), QgsStacObject::Type::Item );
QVERIFY( item );
QCOMPARE( item->id(), QLatin1String( "20201211_223832_CS2" ) );
@ -276,12 +273,12 @@ void TestQgsStac::testFetchStacObjectAsync()
QCOMPARE( spy.at( 0 ).at( 0 ).toInt(), id );
QVERIFY( spy.at( 0 ).at( 1 ).toString().isEmpty() );
std::unique_ptr< QgsStacObject > obj = c.takeStacObject( id );
std::unique_ptr< QgsStacCatalog > obj = c.takeStacObject< QgsStacCatalog >( id );
QVERIFY( obj );
QCOMPARE( obj->type(), QgsStacObject::Type::Catalog );
// cannot take same id twice
obj = c.takeStacObject( id );
obj = c.takeStacObject< QgsStacCatalog >( id );
QVERIFY( !obj );
@ -297,12 +294,12 @@ void TestQgsStac::testFetchStacObjectAsync()
QCOMPARE( spy.at( 0 ).at( 0 ).toInt(), id );
QVERIFY( spy.at( 0 ).at( 1 ).toString().isEmpty() );
obj = c.takeStacObject( id );
obj = c.takeStacObject< QgsStacCatalog >( id );
QVERIFY( obj );
QCOMPARE( obj->type(), QgsStacObject::Type::Collection );
// cannot take same id twice
obj = c.takeStacObject( id );
obj = c.takeStacObject< QgsStacCatalog >( id );
QVERIFY( obj == nullptr );
@ -318,13 +315,13 @@ void TestQgsStac::testFetchStacObjectAsync()
QCOMPARE( spy.at( 0 ).at( 0 ).toInt(), id );
QVERIFY( spy.at( 0 ).at( 1 ).toString().isEmpty() );
obj = c.takeStacObject( id );
QVERIFY( obj );
QCOMPARE( obj->type(), QgsStacObject::Type::Item );
std::unique_ptr< QgsStacItem > item = c.takeStacObject< QgsStacItem >( id );
QVERIFY( item );
QCOMPARE( item->type(), QgsStacObject::Type::Item );
// cannot take same id twice
obj = c.takeStacObject( id );
QVERIFY( !obj );
item = c.takeStacObject< QgsStacItem >( id );
QVERIFY( !item );
}
void TestQgsStac::testFetchItemCollectionAsync()
@ -413,7 +410,7 @@ void TestQgsStac::testFetchStacObjectAsyncInvalid()
// Error should not be empty on failure
QVERIFY( !spy.at( 0 ).at( 1 ).toString().isEmpty() );
QVERIFY( !c.takeStacObject( id ) );
QVERIFY( !c.takeStacObject< QgsStacObject >( id ) );
}
void TestQgsStac::testFetchItemCollectionAsyncInvalid()
@ -473,7 +470,7 @@ void TestQgsStac::testFetchStacObjectAsyncUnavailable()
// Error should not be empty on failure
QVERIFY( !spy.at( 0 ).at( 1 ).toString().isEmpty() );
QVERIFY( !c.takeStacObject( id ) );
QVERIFY( !c.takeStacObject< QgsStacObject >( id ) );
}
void TestQgsStac::testFetchItemCollectionAsyncUnavailable()