Merge pull request #787 from hsorby/is_resolved

Improve ImportedEntity isResolved method. Closes #786.

src/api/libcellml/component.h

@@ -429,9 +429,9 @@ class LIBCELLML_EXPORT Component: public ComponentEntity, public ImportedEntity
 
 private:
     Component(); /**< Constructor @private*/
-    explicit Component(const std::string &name); /**< Constructor named @private */
+    explicit Component(const std::string &name); /**< Constructor named, @private. */
 
-    bool doAddComponent(const ComponentPtr &component) override; /**< Virtual method for implementing addComponent, @private */
+    bool doAddComponent(const ComponentPtr &component) override; /**< Virtual method for implementing addComponent, @private. */
 
     /**
      * @brief Set the import source of this component.
@@ -442,8 +442,10 @@ class LIBCELLML_EXPORT Component: public ComponentEntity, public ImportedEntity
      */
     void doSetImportSource(const ImportSourcePtr &importSource) override;
 
-    struct ComponentImpl; /**< Forward declaration for pImpl idiom. @private */
-    ComponentImpl *mPimpl; /**< Private member to implementation pointer. @private */
+    bool doIsResolved() const override; /**< Virtual method for implementing isResolved, @private. */
+
+    struct ComponentImpl; /**< Forward declaration for pImpl idiom, @private. */
+    ComponentImpl *mPimpl; /**< Private member to implementation pointer, @private. */
 };
 
 } // namespace libcellml

src/api/libcellml/importedentity.h

@@ -35,7 +35,6 @@ class LIBCELLML_EXPORT ImportedEntity
     virtual ~ImportedEntity(); /**< Destructor. */
     ImportedEntity(const ImportedEntity &rhs) = delete; /**< Copy constructor. */
     ImportedEntity(ImportedEntity &&rhs) noexcept = delete; /**< Move constructor. */
-    ImportedEntity &operator=(ImportedEntity rhs) = delete; /**< Assignment operator. */
 
     /**
      * @brief Test if this entity is an imported entity.
@@ -96,25 +95,20 @@ class LIBCELLML_EXPORT ImportedEntity
     void setImportReference(const std::string &reference);
 
     /**
-     * @brief Test whether this imported entity has been resolved.
+     * @brief Test whether this entity has been resolved.
      *
-     * Returns @c true if the import and any dependencies are resolved, otherwise @c false.
+     * Test whether this entity is resolved or not.
+     *
+     * An entity that is not imported is always resolved so this method
+     * returns @c true. Alternatively, if this entity is imported then
+     * it returns @c true if every entity that this imported entity requires
+     * can be found. That is, return @c true if this imported entity is resolvable.
+     * In all other cases, this method returns @c false.
      *
      * @return @c true if the import is resolved, @c false otherwise.
      */
     bool isResolved() const;
 
-    /**
-     * @brief Set the resolution status of this imported entity.
-     *
-     * Set the resolution status of this imported entity.  When @c true, this 
-     * indicates that the item and all its dependent children have been resolved.
-     * Otherwise, @c false.
-     * 
-     * @param status A boolean indicating import resolution status.
-     */
-    void setResolved(bool status);
-
 protected:
     ImportedEntity(); /**< Constructor. */
 
@@ -128,6 +122,16 @@ class LIBCELLML_EXPORT ImportedEntity
      */
     virtual void doSetImportSource(const ImportSourcePtr &importSource);
 
+    /**
+     * @brief Virtual is resolved method to be implemented by derived classes.
+     *
+     * Virtual isResolved method to allow the @ref Units and @ref Component classes to
+     * implement their own versions.
+     *
+     * @return @c true if this imported entity is resolved and @c false otherwise.
+     */
+    virtual bool doIsResolved() const = 0;
+
 private:
     struct ImportedEntityImpl; /**< Forward declaration for pImpl idiom. */
     ImportedEntityImpl *mPimpl; /**< Private member to implementation pointer. */

src/api/libcellml/importer.h

@@ -69,6 +69,7 @@ class LIBCELLML_EXPORT Importer: public Logger
      * Resolve all @c Component and @c Units imports by loading the models
      * from local disk through relative URLs.  The @p baseFile is used to determine
      * the full path to the source model relative to this one.
+     *
      * @param model The @c Model whose imports need resolution.
      * @param baseFile The @c std::string location on local disk of the source @c Model.
      * 

src/api/libcellml/importsource.h

@@ -96,6 +96,13 @@ class LIBCELLML_EXPORT ImportSource: public Entity
      */
     void setModel(const ModelPtr &model);
 
+    /**
+     * @brief Remove the model from this import source.
+     *
+     * Remove the reference to a model from this import source.
+     */
+    void removeModel();
+
     /**
      * @brief Test if this @c ImportSource is resolved.
      *

src/api/libcellml/issue.h

@@ -213,6 +213,7 @@ class LIBCELLML_EXPORT Issue
 
         // Issues not present in the normative specification:
         MAP_VARIABLES_IDENTICAL_UNIT_REDUCTION,
+        INVALID_ARGUMENT,
 
         // Analyser issues:
 

src/api/libcellml/units.h

@@ -529,6 +529,8 @@ class LIBCELLML_EXPORT Units: public NamedEntity, public ImportedEntity
      */
     void doSetImportSource(const ImportSourcePtr &importSource) override;
 
+    bool doIsResolved() const override; /**< Virtual method for implementing isResolved, @private. */
+
     struct UnitsImpl; /**< Forward declaration for pImpl idiom. */
     UnitsImpl *mPimpl; /**< Private member to implementation pointer. */
 };

src/bindings/interface/importsource.i

@@ -28,6 +28,9 @@ unset).";
 %feature("docstring") libcellml::ImportSource::hasModel
 "Returns True if this ImportSource has been resolved, False otherwise.";
 
+%feature("docstring") libcellml::ImportSource::removeModel
+"Remove the model from this ImportSource.";
+
 %feature("docstring") libcellml::ImportSource::clone
 "Create a copy of this import source.";
 

src/component.cpp

@@ -450,4 +450,27 @@ bool Component::requiresImports()
     return doRequiresImport(shared_from_this());
 }
 
+bool Component::doIsResolved() const
+{
+    bool resolved = true;
+    if (isImport()) {
+        auto model = importSource()->model();
+        if (model == nullptr) {
+            resolved = false;
+        } else {
+            auto importedComponent = model->component(importReference());
+            if (importedComponent == nullptr) {
+                resolved = false;
+            } else {
+                resolved = importedComponent->isResolved();
+            }
+        }
+    }
+    for (size_t i = 0; (i < componentCount()) && resolved; ++i) {
+        resolved = component(i)->isResolved();
+    }
+
+    return resolved;
+}
+
 } // namespace libcellml

src/importedentity.cpp

@@ -27,7 +27,6 @@ struct ImportedEntity::ImportedEntityImpl
 {
     ImportSourcePtr mImportSource;
     std::string mImportReference;
-    bool mIsResolved = false;
 };
 
 ImportedEntity::ImportedEntity()
@@ -72,12 +71,7 @@ void ImportedEntity::setImportReference(const std::string &reference)
 
 bool ImportedEntity::isResolved() const
 {
-    return mPimpl->mIsResolved;
-}
-
-void ImportedEntity::setResolved(bool status)
-{
-    mPimpl->mIsResolved = status;
+    return doIsResolved();
 }
 
 } // namespace libcellml

src/importer.cpp

@@ -195,7 +195,7 @@ bool Importer::ImporterImpl::fetchComponent(const ComponentPtr &importComponent,
     // If not, check for model, and parse/instantiate/add to library if needed.
     // If model exists, resolve component's requirements, including child components and units required.
 
-    if (!importComponent->requiresImports() || importComponent->isResolved()) {
+    if (!importComponent->requiresImports()) {
         return true;
     }
 
@@ -259,13 +259,12 @@ bool Importer::ImporterImpl::fetchComponent(const ComponentPtr &importComponent,
         return false;
     }
 
-    importComponent->setResolved(true);
     return true;
 }
 
 bool Importer::ImporterImpl::fetchUnits(const UnitsPtr &importUnits, const std::string &baseFile, std::vector<std::tuple<std::string, std::string, std::string>> &history)
 {
-    if (!importUnits->isImport() || importUnits->isResolved()) {
+    if (!importUnits->isImport()) {
         return true;
     }
 
@@ -320,7 +319,6 @@ bool Importer::ImporterImpl::fetchUnits(const UnitsPtr &importUnits, const std::
         return false;
     }
 
-    importUnits->setResolved(true);
     return true;
 }
 
@@ -380,8 +378,7 @@ bool Importer::resolveImports(ModelPtr &model, const std::string &baseFile)
 void clearComponentImports(const ComponentPtr &component)
 {
     if (component->isImport()) {
-        component->importSource()->setModel(nullptr);
-        component->setResolved(false);
+        component->importSource()->removeModel();
     }
     for (size_t c = 0; c < component->componentCount(); ++c) {
         clearComponentImports(component->component(c));
@@ -394,8 +391,7 @@ void Importer::clearImports(ModelPtr &model)
     for (size_t u = 0; u < model->unitsCount(); ++u) {
         auto mu = model->units(u);
         if (mu->isImport()) {
-            mu->importSource()->setModel(nullptr);
-            mu->setResolved(false);
+            mu->importSource()->removeModel();
         }
     }
     for (size_t c = 0; c < model->componentCount(); ++c) {
@@ -515,10 +511,20 @@ void flattenComponentTree(const ComponentEntityPtr &parent, ComponentPtr &compon
 
 ModelPtr Importer::flattenModel(const ModelPtr &model)
 {
+    ModelPtr flatModel;
+    if (model == nullptr) {
+        auto issue = Issue::create();
+        issue->setReferenceRule(Issue::ReferenceRule::INVALID_ARGUMENT);
+        issue->setDescription("The model is null.");
+        addIssue(issue);
+
+        return flatModel;
+    }
     if (model->hasUnresolvedImports()) {
-        return nullptr;
+        return flatModel;
     }
-    auto flatModel = model->clone();
+
+    flatModel = model->clone();
 
     while (flatModel->hasImports()) {
         // Go through Units and instantiate any imported Units.

src/importsource.cpp

@@ -89,6 +89,11 @@ void ImportSource::setModel(const ModelPtr &model)
     }
 }
 
+void ImportSource::removeModel()
+{
+    mPimpl->mModel.reset();
+}
+
 bool ImportSource::hasModel() const
 {
     return !mPimpl->mModel.expired();

src/model.cpp

@@ -352,67 +352,22 @@ bool Model::hasUnlinkedUnits()
     return unlinkedUnits;
 }
 
-bool isUnresolvedImport(const ImportedEntityPtr &importedEntity)
-{
-    bool unresolvedImport = false;
-    if (importedEntity->isImport()) {
-        ImportSourcePtr importedSource = importedEntity->importSource();
-        unresolvedImport = !importedSource->hasModel();
-    }
-    return unresolvedImport;
-}
-
-bool hasUnresolvedComponentImports(const ComponentEntityConstPtr &parentComponentEntity);
-
-bool doHasUnresolvedComponentImports(const ComponentPtr &component)
-{
-    bool unresolvedImports = false;
-    if (component->isImport()) {
-        unresolvedImports = isUnresolvedImport(component);
-        if (!unresolvedImports) {
-            // Check that the imported component can import all it needs from its model.
-            auto importedSource = component->importSource();
-            auto importedModel = importedSource->model();
-            auto importedComponent = importedModel->component(component->importReference());
-            if (importedComponent == nullptr) {
-                unresolvedImports = true;
-            } else {
-                unresolvedImports = doHasUnresolvedComponentImports(importedComponent);
-            }
-        }
-    } else {
-        unresolvedImports = hasUnresolvedComponentImports(component);
-    }
-    return unresolvedImports;
-}
-
-bool hasUnresolvedComponentImports(const ComponentEntityConstPtr &parentComponentEntity)
-{
-    bool unresolvedImports = false;
-    for (size_t n = 0; n < parentComponentEntity->componentCount() && !unresolvedImports; ++n) {
-        libcellml::ComponentPtr component = parentComponentEntity->component(n);
-        unresolvedImports = doHasUnresolvedComponentImports(component);
-    }
-    return unresolvedImports;
-}
-
 bool Model::hasUnresolvedImports() const
 {
     bool unresolvedImports = false;
     for (size_t n = 0; n < unitsCount() && !unresolvedImports; ++n) {
-        libcellml::UnitsPtr units = Model::units(n);
-        unresolvedImports = isUnresolvedImport(units);
+        unresolvedImports = !units(n)->isResolved();
     }
-    if (!unresolvedImports) {
-        unresolvedImports = hasUnresolvedComponentImports(shared_from_this());
+    for (size_t n = 0; (n < componentCount()) && !unresolvedImports; ++n) {
+        unresolvedImports = !component(n)->isResolved();
     }
     return unresolvedImports;
 }
 
 bool hasComponentImports(const ComponentEntityConstPtr &componentEntity)
 {
     bool importsPresent = false;
-    for (size_t n = 0; n < componentEntity->componentCount() && !importsPresent; ++n) {
+    for (size_t n = 0; (n < componentEntity->componentCount()) && !importsPresent; ++n) {
         libcellml::ComponentPtr childComponent = componentEntity->component(n);
         importsPresent = childComponent->isImport();
         if (!importsPresent) {
@@ -425,7 +380,7 @@ bool hasComponentImports(const ComponentEntityConstPtr &componentEntity)
 bool Model::hasImports() const
 {
     bool importsPresent = false;
-    for (size_t n = 0; n < unitsCount() && !importsPresent; ++n) {
+    for (size_t n = 0; (n < unitsCount()) && !importsPresent; ++n) {
         libcellml::UnitsPtr units = Model::units(n);
         if (units->isImport()) {
             importsPresent = true;

src/units.cpp

@@ -666,4 +666,43 @@ UnitsPtr Units::clone() const
     return units;
 }
 
+bool Units::doIsResolved() const
+{
+    bool resolved = true;
+    if (isImport()) {
+        auto model = importSource()->model();
+        if (model == nullptr) {
+            resolved = false;
+        } else {
+            auto importedUnits = model->units(importReference());
+            if (importedUnits == nullptr) {
+                resolved = false;
+            } else {
+                if (importedUnits->isImport()) {
+                    resolved = importedUnits->isResolved();
+                } else {
+                    for (size_t u = 0; (u < importedUnits->unitCount()) && resolved; ++u) {
+                        std::string reference;
+                        std::string prefix;
+                        std::string id;
+                        double exponent;
+                        double multiplier;
+                        importedUnits->unitAttributes(u, reference, prefix, exponent, multiplier, id);
+                        if (isStandardUnitName(reference)) {
+                            continue;
+                        }
+                        auto childUnits = model->units(reference);
+                        if (childUnits == nullptr) {
+                            resolved = false;
+                        } else {
+                            resolved = childUnits->isResolved();
+                        }
+                    }
+                }
+            }
+        }
+    }
+    return resolved;
+}
+
 } // namespace libcellml