diff --git a/src/core/stac/qgsstaccontroller.cpp b/src/core/stac/qgsstaccontroller.cpp index 762f8ed574d..e525ba93e26 100644 --- a/src/core/stac/qgsstaccontroller.cpp +++ b/src/core/stac/qgsstaccontroller.cpp @@ -17,6 +17,7 @@ #include "moc_qgsstaccontroller.cpp" #include "qgsstaccatalog.h" #include "qgsstaccollection.h" +#include "qgsstaccollections.h" #include "qgsstacitem.h" #include "qgsstacitemcollection.h" #include "qgsstacparser.h" @@ -213,22 +214,25 @@ void QgsStacController::handleCollectionsReply() mReplies.removeOne( reply ); } -QgsStacObject *QgsStacController::takeStacObject( int requestId ) +std::unique_ptr< QgsStacObject > QgsStacController::takeStacObject( int requestId ) { - return mFetchedStacObjects.take( requestId ); + std::unique_ptr< QgsStacObject > obj( mFetchedStacObjects.take( requestId ) ); + return obj; } -QgsStacItemCollection *QgsStacController::takeItemCollection( int requestId ) +std::unique_ptr< QgsStacItemCollection > QgsStacController::takeItemCollection( int requestId ) { - return mFetchedItemCollections.take( requestId ); + std::unique_ptr< QgsStacItemCollection > col( mFetchedItemCollections.take( requestId ) ); + return col; } -QgsStacCollections *QgsStacController::takeCollections( int requestId ) +std::unique_ptr< QgsStacCollections > QgsStacController::takeCollections( int requestId ) { - return mFetchedCollections.take( requestId ); + std::unique_ptr< QgsStacCollections > cols( mFetchedCollections.take( requestId ) ); + return cols; } -QgsStacObject *QgsStacController::fetchStacObject( const QUrl &url, QString *error ) +std::unique_ptr< QgsStacObject > QgsStacController::fetchStacObject( const QUrl &url, QString *error ) { QgsNetworkReplyContent content = fetchBlocking( url ); @@ -245,20 +249,19 @@ QgsStacObject *QgsStacController::fetchStacObject( const QUrl &url, QString *err QgsStacParser parser; parser.setData( data ); parser.setBaseUrl( url ); - QgsStacObject *object = nullptr; + std::unique_ptr< QgsStacObject > object; switch ( parser.type() ) { case QgsStacObject::Type::Catalog: - object = parser.catalog(); + object.reset( parser.catalog() ); break; case QgsStacObject::Type::Collection: - object = parser.collection(); + object.reset( parser.collection() ); break; case QgsStacObject::Type::Item: - object = parser.item(); + object.reset( parser.item() ); break; case QgsStacObject::Type::Unknown: - object = nullptr; break; } @@ -268,7 +271,7 @@ QgsStacObject *QgsStacController::fetchStacObject( const QUrl &url, QString *err return object; } -QgsStacItemCollection *QgsStacController::fetchItemCollection( const QUrl &url, QString *error ) +std::unique_ptr< QgsStacItemCollection > QgsStacController::fetchItemCollection( const QUrl &url, QString *error ) { QgsNetworkReplyContent content = fetchBlocking( url ); @@ -285,7 +288,7 @@ QgsStacItemCollection *QgsStacController::fetchItemCollection( const QUrl &url, QgsStacParser parser; parser.setData( data ); parser.setBaseUrl( url ); - QgsStacItemCollection *ic = parser.itemCollection(); + std::unique_ptr< QgsStacItemCollection > ic( parser.itemCollection() ); if ( error ) *error = parser.error(); @@ -293,7 +296,7 @@ QgsStacItemCollection *QgsStacController::fetchItemCollection( const QUrl &url, return ic; } -QgsStacCollections *QgsStacController::fetchCollections( const QUrl &url, QString *error ) +std::unique_ptr< QgsStacCollections > QgsStacController::fetchCollections( const QUrl &url, QString *error ) { QgsNetworkReplyContent content = fetchBlocking( url ); @@ -309,7 +312,7 @@ QgsStacCollections *QgsStacController::fetchCollections( const QUrl &url, QStrin QgsStacParser parser; parser.setData( data ); - QgsStacCollections *col = parser.collections(); + std::unique_ptr< QgsStacCollections > col( parser.collections() ); if ( error ) *error = parser.error(); diff --git a/src/core/stac/qgsstaccontroller.h b/src/core/stac/qgsstaccontroller.h index acf8e86a844..b53021f8cfd 100644 --- a/src/core/stac/qgsstaccontroller.h +++ b/src/core/stac/qgsstaccontroller.h @@ -75,21 +75,21 @@ class CORE_EXPORT QgsStacController : public QObject * An optional \a error parameter will be populated with any network error information. * The caller takes ownership of the returned object */ - QgsStacObject *fetchStacObject( const QUrl &url, QString *error = nullptr ); + std::unique_ptr< QgsStacObject > fetchStacObject( const QUrl &url, QString *error = nullptr ); /** * Fetches a feature collection 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 feature collection */ - QgsStacItemCollection *fetchItemCollection( const QUrl &url, QString *error = nullptr ); + std::unique_ptr< QgsStacItemCollection > fetchItemCollection( const QUrl &url, QString *error = nullptr ); /** * Fetches collections 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 feature collection */ - QgsStacCollections *fetchCollections( const QUrl &url, QString *error = nullptr ); + std::unique_ptr< QgsStacCollections > fetchCollections( const QUrl &url, QString *error = nullptr ); /** * Initiates an asynchronous request for a STAC object using the \a url @@ -130,7 +130,7 @@ class CORE_EXPORT QgsStacController : public QObject * \see fetchStacObjectAsync * \see finishedStacObjectRequest */ - QgsStacObject *takeStacObject( int requestId ); + std::unique_ptr< QgsStacObject > takeStacObject( int requestId ); /** * Returns the feature collection fetched with the specified \a requestId @@ -140,7 +140,7 @@ class CORE_EXPORT QgsStacController : public QObject * \see fetchItemCollectionAsync * \see finishedItemCollectionRequest */ - QgsStacItemCollection *takeItemCollection( int requestId ); + std::unique_ptr< QgsStacItemCollection > takeItemCollection( int requestId ); /** * Returns the collections collection fetched with the specified \a requestId @@ -151,7 +151,7 @@ class CORE_EXPORT QgsStacController : public QObject * \see finishedCollectionsRequest * \since QGIS 3.42 */ - QgsStacCollections *takeCollections( int requestId ); + std::unique_ptr< QgsStacCollections > takeCollections( int requestId ); /** * Returns the authentication config id which will be used during the request. diff --git a/src/core/stac/qgsstacdataitems.cpp b/src/core/stac/qgsstacdataitems.cpp index 08e0df860d0..8ae76c336cb 100644 --- a/src/core/stac/qgsstacdataitems.cpp +++ b/src/core/stac/qgsstacdataitems.cpp @@ -69,9 +69,8 @@ QVector QgsStacItemItem::createChildren() { QgsStacController *controller = stacController(); QString error; - QgsStacObject *obj = controller->fetchStacObject( mPath, &error ); - QgsStacItem *item = dynamic_cast( obj ); - setStacItem( item ); + std::unique_ptr< QgsStacObject > obj = controller->fetchStacObject( mPath, &error ); + setStacItem( obj ); if ( !mStacItem ) return { new QgsErrorItem( this, error, path() + QStringLiteral( "/error" ) ) }; @@ -175,8 +174,12 @@ QgsStacController *QgsStacItemItem::stacController() return nullptr; } -void QgsStacItemItem::setStacItem( QgsStacItem *item ) +void QgsStacItemItem::setStacItem( std::unique_ptr< QgsStacObject > &object ) { + QgsStacItem *item = dynamic_cast( object.get() ); + if ( item ) + object.release(); + mStacItem.reset( item ); updateToolTip(); } @@ -189,20 +192,18 @@ QgsStacItem *QgsStacItemItem::stacItem() const void QgsStacItemItem::itemRequestFinished( int requestId, QString error ) { QgsStacController *controller = stacController(); - QgsStacObject *object = controller->takeStacObject( requestId ); - QgsStacItem *item = dynamic_cast< QgsStacItem * >( object ); - setStacItem( item ); - if ( item ) + std::unique_ptr< QgsStacObject > object = controller->takeStacObject( requestId ); + setStacItem( object ); + if ( mStacItem ) { mIconName = QStringLiteral( "mActionPropertiesWidget.svg" ); - QString name = item->properties().value( QStringLiteral( "title" ), QString() ).toString(); + QString name = mStacItem->properties().value( QStringLiteral( "title" ), QString() ).toString(); if ( name.isEmpty() ) - name = item->id(); + name = mStacItem->id(); mName = name; } else { - delete object; mIconName = QStringLiteral( "/mIconDelete.svg" ); mName = error; } @@ -318,9 +319,8 @@ QVector QgsStacCatalogItem::createChildren() QgsStacController *controller = stacController(); QString error; - QgsStacObject *obj = controller->fetchStacObject( mPath, &error ); - QgsStacCatalog *cat = dynamic_cast( obj ); - setStacCatalog( cat ); + std::unique_ptr< QgsStacObject > obj = controller->fetchStacObject( mPath, &error ); + setStacCatalog( obj ); if ( !mStacCatalog ) return { new QgsErrorItem( this, error, path() + QStringLiteral( "/error" ) ) }; @@ -459,8 +459,12 @@ void QgsStacCatalogItem::updateToolTip() } } -void QgsStacCatalogItem::setStacCatalog( QgsStacCatalog *catalog ) +void QgsStacCatalogItem::setStacCatalog( std::unique_ptr< QgsStacObject > &object ) { + QgsStacCatalog *catalog = dynamic_cast( object.get() ); + if ( catalog ) + object.release(); + mStacCatalog.reset( catalog ); if ( mStacCatalog ) { @@ -490,10 +494,12 @@ QVector< QgsDataItem * > QgsStacCatalogItem::createItems( const QVector object( item ); + const QString name = item->properties().value( QStringLiteral( "title" ), item->id() ).toString(); QgsStacItemItem *i = new QgsStacItemItem( this, name, item->url() ); - i->setStacItem( item ); + i->setStacItem( object ); i->setState( Qgis::BrowserItemState::Populated ); contents.append( i ); } @@ -509,10 +515,12 @@ QVector QgsStacCatalogItem::createCollections( const QVector object( col ); + const QString name = col->title().isEmpty() ? col->id() : col->title(); QgsStacCatalogItem *i = new QgsStacCatalogItem( this, name, col->url() ); - i->setStacCatalog( col ); + i->setStacCatalog( object ); contents.append( i ); } return contents; diff --git a/src/core/stac/qgsstacdataitems.h b/src/core/stac/qgsstacdataitems.h index 658a0cdd328..89f947c48d8 100644 --- a/src/core/stac/qgsstacdataitems.h +++ b/src/core/stac/qgsstacdataitems.h @@ -59,7 +59,7 @@ class CORE_EXPORT QgsStacItemItem : public QgsDataItem QgsStacController *stacController(); //! takes ownership - void setStacItem( QgsStacItem *item ); + void setStacItem( std::unique_ptr< QgsStacObject > &object ); //! does not transfer ownership QgsStacItem *stacItem() const; @@ -87,7 +87,7 @@ class CORE_EXPORT QgsStacCatalogItem : public QgsDataCollectionItem void updateToolTip(); //! takes ownership - void setStacCatalog( QgsStacCatalog *catalog ); + void setStacCatalog( std::unique_ptr< QgsStacObject > &object ); //! does not transfer ownership QgsStacCatalog *stacCatalog() const; diff --git a/tests/src/core/testqgsstac.cpp b/tests/src/core/testqgsstac.cpp index a4cb32205dc..cda45f48d8c 100644 --- a/tests/src/core/testqgsstac.cpp +++ b/tests/src/core/testqgsstac.cpp @@ -86,10 +86,10 @@ void TestQgsStac::testParseLocalCatalog() { const QUrl url( QStringLiteral( "file://%1%2" ).arg( mDataDir, QStringLiteral( "catalog.json" ) ) ); QgsStacController c; - QgsStacObject *obj = c.fetchStacObject( url.toString() ); + std::unique_ptr< QgsStacObject > obj = c.fetchStacObject( url.toString() ); QVERIFY( obj ); QCOMPARE( obj->type(), QgsStacObject::Type::Catalog ); - QgsStacCatalog *cat = dynamic_cast( obj ); + QgsStacCatalog *cat = dynamic_cast( obj.get() ); QVERIFY( cat ); QCOMPARE( cat->id(), QLatin1String( "examples" ) ); @@ -108,18 +108,16 @@ void TestQgsStac::testParseLocalCatalog() QCOMPARE( links.at( 3 ).href(), QStringLiteral( "%1collection-only/collection-with-schemas.json" ).arg( basePath ) ); QCOMPARE( links.at( 4 ).href(), QStringLiteral( "%1collectionless-item.json" ).arg( basePath ) ); QCOMPARE( links.at( 5 ).href(), QStringLiteral( "https://raw.githubusercontent.com/radiantearth/stac-spec/v1.0.0/examples/catalog.json" ) ); - - delete cat; } void TestQgsStac::testParseLocalCollection() { const QUrl url( QStringLiteral( "file://%1%2" ).arg( mDataDir, QStringLiteral( "collection.json" ) ) ); QgsStacController c; - QgsStacObject *obj = c.fetchStacObject( url.toString() ); + std::unique_ptr< QgsStacObject > obj = c.fetchStacObject( url.toString() ); QVERIFY( obj ); QCOMPARE( obj->type(), QgsStacObject::Type::Collection ); - QgsStacCollection *col = dynamic_cast( obj ); + QgsStacCollection *col = dynamic_cast( obj.get() ); QVERIFY( col ); QCOMPARE( col->id(), QLatin1String( "simple-collection" ) ); @@ -158,18 +156,16 @@ void TestQgsStac::testParseLocalCollection() QVariantMap sum( col->summaries() ); QCOMPARE( sum.size(), 9 ); QCOMPARE( sum.value( QStringLiteral( "platform" ) ).toStringList(), QStringList() << QLatin1String( "cool_sat1" ) << QLatin1String( "cool_sat2" ) ); - - delete col; } void TestQgsStac::testParseLocalItem() { const QUrl url( QStringLiteral( "file://%1%2" ).arg( mDataDir, QStringLiteral( "core-item.json" ) ) ); QgsStacController c; - QgsStacObject *obj = c.fetchStacObject( url.toString() ); + std::unique_ptr< QgsStacObject > obj = c.fetchStacObject( url.toString() ); QVERIFY( obj ); QCOMPARE( obj->type(), QgsStacObject::Type::Item ); - QgsStacItem *item = dynamic_cast( obj ); + QgsStacItem *item = dynamic_cast( obj.get() ); QVERIFY( item ); QCOMPARE( item->id(), QLatin1String( "20201211_223832_CS2" ) ); @@ -221,15 +217,13 @@ void TestQgsStac::testParseLocalItem() QVERIFY( !uri.isValid() ); QVERIFY( uri.uri.isEmpty() ); QVERIFY( uri.name.isEmpty() ); - - delete item; } void TestQgsStac::testParseLocalItemCollection() { const QString fileName = mDataDir + QStringLiteral( "itemcollection-sample-full.json" ); QgsStacController c; - QgsStacItemCollection *ic = c.fetchItemCollection( QStringLiteral( "file://%1" ).arg( fileName ) ); + std::unique_ptr< QgsStacItemCollection > ic = c.fetchItemCollection( QStringLiteral( "file://%1" ).arg( fileName ) ); QVERIFY( ic ); QCOMPARE( ic->numberReturned(), 1 ); QCOMPARE( ic->numberMatched(), 10 ); @@ -242,15 +236,13 @@ void TestQgsStac::testParseLocalItemCollection() QCOMPARE( items.first()->links().size(), 3 ); QCOMPARE( items.first()->stacExtensions().size(), 0 ); QCOMPARE( items.first()->assets().size(), 2 ); - - delete ic; } void TestQgsStac::testParseLocalCollections() { const QString fileName = mDataDir + QStringLiteral( "collectioncollection-sample-full.json" ); QgsStacController c; - QgsStacCollections *cols = c.fetchCollections( QStringLiteral( "file://%1" ).arg( fileName ) ); + std::unique_ptr< QgsStacCollections > cols = c.fetchCollections( QStringLiteral( "file://%1" ).arg( fileName ) ); QVERIFY( cols ); QCOMPARE( cols->numberReturned(), 1 ); QCOMPARE( cols->numberMatched(), 11 ); @@ -266,8 +258,6 @@ void TestQgsStac::testParseLocalCollections() QCOMPARE( col->stacVersion(), QLatin1String( "1.0.0" ) ); QCOMPARE( col->links().size(), 3 ); QCOMPARE( col->stacExtensions().size(), 0 ); - - delete cols; } void TestQgsStac::testFetchStacObjectAsync() @@ -286,14 +276,13 @@ void TestQgsStac::testFetchStacObjectAsync() QCOMPARE( spy.at( 0 ).at( 0 ).toInt(), id ); QVERIFY( spy.at( 0 ).at( 1 ).toString().isEmpty() ); - QgsStacObject *obj = c.takeStacObject( id ); + std::unique_ptr< QgsStacObject > obj = c.takeStacObject( id ); QVERIFY( obj ); QCOMPARE( obj->type(), QgsStacObject::Type::Catalog ); - delete obj; // cannot take same id twice obj = c.takeStacObject( id ); - QVERIFY( obj == nullptr ); + QVERIFY( !obj ); // Collection @@ -311,7 +300,6 @@ void TestQgsStac::testFetchStacObjectAsync() obj = c.takeStacObject( id ); QVERIFY( obj ); QCOMPARE( obj->type(), QgsStacObject::Type::Collection ); - delete obj; // cannot take same id twice obj = c.takeStacObject( id ); @@ -333,11 +321,10 @@ void TestQgsStac::testFetchStacObjectAsync() obj = c.takeStacObject( id ); QVERIFY( obj ); QCOMPARE( obj->type(), QgsStacObject::Type::Item ); - delete obj; // cannot take same id twice obj = c.takeStacObject( id ); - QVERIFY( obj == nullptr ); + QVERIFY( !obj ); } void TestQgsStac::testFetchItemCollectionAsync() @@ -356,7 +343,7 @@ void TestQgsStac::testFetchItemCollectionAsync() QCOMPARE( spy.at( 0 ).at( 0 ).toInt(), id ); QVERIFY( spy.at( 0 ).at( 1 ).toString().isEmpty() ); - QgsStacItemCollection *ic = c.takeItemCollection( id ); + std::unique_ptr< QgsStacItemCollection > ic = c.takeItemCollection( id ); QVERIFY( ic ); QCOMPARE( ic->numberReturned(), 1 ); QCOMPARE( ic->numberMatched(), 10 ); @@ -369,11 +356,10 @@ void TestQgsStac::testFetchItemCollectionAsync() QCOMPARE( items.first()->links().size(), 3 ); QCOMPARE( items.first()->stacExtensions().size(), 0 ); QCOMPARE( items.first()->assets().size(), 2 ); - delete ic; // cannot take same id twice ic = c.takeItemCollection( id ); - QVERIFY( ic == nullptr ); + QVERIFY( !ic ); } void TestQgsStac::testFetchCollectionsAsync() @@ -392,7 +378,7 @@ void TestQgsStac::testFetchCollectionsAsync() QCOMPARE( spy.at( 0 ).at( 0 ).toInt(), id ); QVERIFY( spy.at( 0 ).at( 1 ).toString().isEmpty() ); - QgsStacCollections *cols = c.takeCollections( id ); + std::unique_ptr< QgsStacCollections > cols = c.takeCollections( id ); QVERIFY( cols ); QCOMPARE( cols->numberReturned(), 1 ); QCOMPARE( cols->numberMatched(), 11 ); @@ -408,8 +394,6 @@ void TestQgsStac::testFetchCollectionsAsync() QCOMPARE( col->stacVersion(), QLatin1String( "1.0.0" ) ); QCOMPARE( col->links().size(), 3 ); QCOMPARE( col->stacExtensions().size(), 0 ); - - delete cols; } void TestQgsStac::testFetchStacObjectAsyncInvalid() @@ -429,7 +413,7 @@ void TestQgsStac::testFetchStacObjectAsyncInvalid() // Error should not be empty on failure QVERIFY( !spy.at( 0 ).at( 1 ).toString().isEmpty() ); - QVERIFY( c.takeStacObject( id ) == nullptr ); + QVERIFY( !c.takeStacObject( id ) ); } void TestQgsStac::testFetchItemCollectionAsyncInvalid() @@ -449,7 +433,7 @@ void TestQgsStac::testFetchItemCollectionAsyncInvalid() // Error should not be empty on failure QVERIFY( !spy.at( 0 ).at( 1 ).toString().isEmpty() ); - QVERIFY( c.takeItemCollection( id ) == nullptr ); + QVERIFY( !c.takeItemCollection( id ) ); } void TestQgsStac::testFetchCollectionsAsyncInvalid() @@ -469,7 +453,7 @@ void TestQgsStac::testFetchCollectionsAsyncInvalid() // Error should not be empty on failure QVERIFY( !spy.at( 0 ).at( 1 ).toString().isEmpty() ); - QVERIFY( c.takeCollections( id ) == nullptr ); + QVERIFY( !c.takeCollections( id ) ); } void TestQgsStac::testFetchStacObjectAsyncUnavailable() @@ -489,7 +473,7 @@ void TestQgsStac::testFetchStacObjectAsyncUnavailable() // Error should not be empty on failure QVERIFY( !spy.at( 0 ).at( 1 ).toString().isEmpty() ); - QVERIFY( c.takeStacObject( id ) == nullptr ); + QVERIFY( !c.takeStacObject( id ) ); } void TestQgsStac::testFetchItemCollectionAsyncUnavailable() @@ -509,7 +493,7 @@ void TestQgsStac::testFetchItemCollectionAsyncUnavailable() // Error should not be empty on failure QVERIFY( !spy.at( 0 ).at( 1 ).toString().isEmpty() ); - QVERIFY( c.takeItemCollection( id ) == nullptr ); + QVERIFY( !c.takeItemCollection( id ) ); } void TestQgsStac::testFetchCollectionsAsyncUnavailable() @@ -529,7 +513,7 @@ void TestQgsStac::testFetchCollectionsAsyncUnavailable() // Error should not be empty on failure QVERIFY( !spy.at( 0 ).at( 1 ).toString().isEmpty() ); - QVERIFY( c.takeCollections( id ) == nullptr ); + QVERIFY( !c.takeCollections( id ) ); } QGSTEST_MAIN( TestQgsStac )