From 19d9c5bedd02147d35ac4344cf36b713d154c6f3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= <mosra@centrum.cz>
Date: Thu, 18 May 2023 22:25:28 +0200
Subject: [PATCH] Trade: check that the importer is opened in
 addSupportedImporterContents().

It was done only inside the internal addImporterContents()
implementation this function delegates to which was too late, as the
importer was queried before that already.
---
 src/Magnum/Trade/AbstractSceneConverter.cpp   |  7 +++--
 .../Trade/Test/AbstractSceneConverterTest.cpp | 28 ++++++++++++++++++-
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/src/Magnum/Trade/AbstractSceneConverter.cpp b/src/Magnum/Trade/AbstractSceneConverter.cpp
index fe968e56d..85ff16469 100644
--- a/src/Magnum/Trade/AbstractSceneConverter.cpp
+++ b/src/Magnum/Trade/AbstractSceneConverter.cpp
@@ -1296,8 +1296,6 @@ Containers::Optional<UnsignedInt> AbstractSceneConverter::add(const Containers::
 bool AbstractSceneConverter::addImporterContentsInternal(AbstractImporter& importer, const SceneContents contents, const bool noLevelsIfUnsupported) {
     CORRADE_ASSERT(isConverting(),
         "Trade::AbstractSceneConverter::addImporterContents(): no conversion in progress", {});
-    CORRADE_ASSERT(importer.isOpened(),
-        "Trade::AbstractSceneConverter::addImporterContents(): the importer is not opened", {});
     const SceneContents contentsSupported = sceneContentsFor(*this);
     #ifndef CORRADE_NO_ASSERT
     const SceneContents contentsPresentExceptLevels = contents & sceneContentsFor(importer);
@@ -1640,10 +1638,15 @@ bool AbstractSceneConverter::addImporterContentsInternal(AbstractImporter& impor
 
 
 bool AbstractSceneConverter::addImporterContents(AbstractImporter& importer, const SceneContents contents) {
+    CORRADE_ASSERT(importer.isOpened(),
+        "Trade::AbstractSceneConverter::addImporterContents(): the importer is not opened", {});
     return addImporterContentsInternal(importer, contents, false);
 }
 
 bool AbstractSceneConverter::addSupportedImporterContents(AbstractImporter& importer, const SceneContents contents) {
+    CORRADE_ASSERT(importer.isOpened(),
+        "Trade::AbstractSceneConverter::addSupportedImporterContents(): the importer is not opened", {});
+
     /* To avoid accidental differences in handling SceneConverterFeatures in
        sceneContentsFor(const AbstractSceneConverter&) and here, this branches
        on SceneContents instead of SceneConverterFeatures */
diff --git a/src/Magnum/Trade/Test/AbstractSceneConverterTest.cpp b/src/Magnum/Trade/Test/AbstractSceneConverterTest.cpp
index f494918ef..6bb9c5207 100644
--- a/src/Magnum/Trade/Test/AbstractSceneConverterTest.cpp
+++ b/src/Magnum/Trade/Test/AbstractSceneConverterTest.cpp
@@ -302,6 +302,7 @@ struct AbstractSceneConverterTest: TestSuite::Tester {
 
     void addSupportedImporterContents();
     void addSupportedImporterContentsLevels();
+    void addSupportedImporterContentsNotOpened();
 
     void debugFeature();
     void debugFeaturePacked();
@@ -864,7 +865,8 @@ AbstractSceneConverterTest::AbstractSceneConverterTest() {
     addInstancedTests({&AbstractSceneConverterTest::addSupportedImporterContents},
         Containers::arraySize(AddSupportedImporterContentsData));
 
-    addTests({&AbstractSceneConverterTest::addSupportedImporterContentsLevels,
+    addTests({&AbstractSceneConverterTest::addSupportedImporterContentsNotOpened,
+              &AbstractSceneConverterTest::addSupportedImporterContentsLevels,
 
               &AbstractSceneConverterTest::debugFeature,
               &AbstractSceneConverterTest::debugFeaturePacked,
@@ -7505,6 +7507,30 @@ void AbstractSceneConverterTest::addSupportedImporterContentsLevels() {
     CORRADE_COMPARE(converter.image3DCount(), importer.image3DCount());
 }
 
+void AbstractSceneConverterTest::addSupportedImporterContentsNotOpened() {
+    CORRADE_SKIP_IF_NO_ASSERT();
+
+    struct: AbstractImporter {
+        ImporterFeatures doFeatures() const override { return {}; }
+        bool doIsOpened() const override { return false; }
+        void doClose() override {}
+    } importer;
+
+    struct: AbstractSceneConverter {
+        SceneConverterFeatures doFeatures() const override {
+            return SceneConverterFeature::ConvertMultipleToData;
+        }
+        bool doBeginData() override { return true; }
+    } converter;
+
+    CORRADE_VERIFY(converter.beginData());
+
+    std::ostringstream out;
+    Error redirectError{&out};
+    CORRADE_VERIFY(!converter.addSupportedImporterContents(importer));
+    CORRADE_COMPARE(out.str(), "Trade::AbstractSceneConverter::addSupportedImporterContents(): the importer is not opened\n");
+}
+
 void AbstractSceneConverterTest::debugFeature() {
     std::ostringstream out;