Issue/2000 automatically update gui (#3289)

* Make it possible to query a propertyowner by uri with the getpropertytopic

* Remove automatic reloading of ui from drag and drop script

* Add function for uri for propertyowners

* Add uri to the to_json for the propertyowner

* Add comment for the propertyOwner function and update the comment for the property function to clearer distinguish between uris and identifiers

* Go back to the old events but with uris instead as their member

* Apply feedback from PR

* Address PR comments

---------

Co-authored-by: Ylva Selling <ylva.selling@liu.se>

include/openspace/events/event.h

@@ -123,12 +123,12 @@ struct EventSceneGraphNodeAdded : public Event {
     /**
      * Creates an instance of an EventSceneGraphNodeAdded event.
      *
-     * \param node_ The identifier of the node that was added
+     * \param node_ A pointer to the node that was added
      *
      * \pre node_ must not be nullptr
      */
     explicit EventSceneGraphNodeAdded(const SceneGraphNode* node_);
-    const tstring node;
+    const tstring uri;
 };
 
 /**
@@ -141,12 +141,12 @@ struct EventSceneGraphNodeRemoved : public Event {
     /**
      * Creates an instance of an EventSceneGraphNodeRemoved event.
      *
-     * \param node_ The identifier of the node that was removed
+     * \param node_ A pointer to the node that was removed
      *
      * \pre node_ must not be nullptr
      */
     explicit EventSceneGraphNodeRemoved(const SceneGraphNode* node_);
-    const tstring node;
+    const tstring uri;
 };
 
 /**
@@ -225,7 +225,7 @@ struct EventScreenSpaceRenderableAdded : public Event {
      * \pre renderable_ must not be nullptr
      */
     explicit EventScreenSpaceRenderableAdded(const ScreenSpaceRenderable* renderable_);
-    const tstring renderable;
+    const tstring uri;
 };
 
 /**
@@ -240,9 +240,11 @@ struct EventScreenSpaceRenderableRemoved : public Event {
      * Creates an instance of an EventScreenSpaceRenderableRemoved event.
      *
      * \param renderable_ The the new screenspace renderable that was removed
+     *
+     * \pre renderable_ must not be nullptr
      */
     explicit EventScreenSpaceRenderableRemoved(const ScreenSpaceRenderable* renderable_);
-    const tstring renderable;
+    const tstring uri;
 };
 
 /**
@@ -397,20 +399,13 @@ struct EventLayerAdded : public Event {
     /**
      * Creates an instance of an EventLayerAdded event.
      *
-     * \param node_ The identifier of the globe to which the layer is added
-     * \param layerGroup_ The identifier of the layer group to which the layer is added
-     * \param layer_ The identifier of the layer that was added
+     * \param uri_ A string with the uri of the layer that was added
      *
-     * \pre node_ must not be empty
-     * \pre layerGroup_ must not be empty
-     * \pre layer_ must not be empty
+     * \pre uri_ must be a valid uri
      */
-    explicit EventLayerAdded(std::string_view node_, std::string_view layerGroup_,
-        std::string_view layer_);
+    explicit EventLayerAdded(std::string_view uri_);
 
-    const tstring node;
-    const tstring layerGroup;
-    const tstring layer;
+    const tstring uri;
 };
 
 /**
@@ -422,20 +417,13 @@ struct EventLayerRemoved : public Event {
     /**
      * Creates an instance of an EventLayerRemoved event.
      *
-     * \param node_ The identifier of the globe to which the layer is removed
-     * \param layerGroup_ The identifier of the layer group to which the layer is removed
-     * \param layer_ The identifier of the layer that was removed
+     * \param uri_ The uri of the layer that was removed
      *
-     * \pre node_ must not be empty
-     * \pre layerGroup_ must not be empty
-     * \pre layer_ must not be empty
+     * \pre uri_ must be a valid uri
      */
-    explicit EventLayerRemoved(std::string_view node_, std::string_view layerGroup_,
-        std::string_view layer_);
+    explicit EventLayerRemoved(std::string_view uri_);
 
-    const tstring node;
-    const tstring layerGroup;
-    const tstring layer;
+    const tstring uri;
 };
 
 /**

include/openspace/properties/propertyowner.h

@@ -140,18 +140,41 @@ public:
 
     /**
      * Retrieves a Property identified by \p uri from this PropertyOwner. If \p uri does
-     * not contain a `.` the identifier must refer to a Property directly owned by this
-     * PropertyOwner. If the identifier contains one or more `.`, the first part of the
-     * name will be recursively extracted and used as a name for a sub-owner and only the
-     * last part of the identifier is referring to a Property owned by PropertyOwner named
-     * by the second-but-last name.
+     * not contain a `.`  it is an identifier and must refer to a Property directly owned
+     * by this PropertyOwner. If the identifier contains one or more `.`, the first part
+     * of the name will be recursively extracted and used as a name for a sub-owner and
+     * only the last part of the identifier is referring to a Property owned by a
+     * PropertyOwner named by the second-but-last name.
      *
-     * \param uri The identifier of the Property that should be extracted
+     * \param uri The uri or identifier of the Property that should be extracted
      * \return If the Property cannot be found, `nullptr` is returned, otherwise the
      *         pointer to the Property is returned
      */
     Property* property(const std::string& uri) const;
 
+    /**
+     * Retrieves a PropertyOwner identified by \p uri from this PropertyOwner. If \p uri 
+     * does not contain a `.` it is an identifier and must refer to a PropertyOwner
+     * directly owned by this PropertyOwner. If the uri contains one or more `.`, the
+     * first part of the name will be recursively extracted and used as a name for a sub-
+     * owner and only the last part of the uri is referring to a PropertyOwner owned by a 
+     * PropertyOwner named by the second-but-last name.
+     *
+     * \param uri The uri or identifier of the PropertyOwner that should be extracted
+     * \return If the PropertyOwner cannot be found, `nullptr` is returned, otherwise the
+     *         pointer to the PropertyOwner is returned
+     */
+    PropertyOwner* propertyOwner(const std::string& uri) const;
+
+    /**
+     * Returns a uri for this PropertyOwner. This is created by looking up all the owners
+     * of this PropertyOwner. The owner identifiers are separated by ".", which make up
+     * the uri of this PropertyOwner.
+     * 
+     * \return The uri of this PropertyOwner 
+     */
+    std::string uri() const;
+
     /**
      * This method checks if a Property with the provided \p uri exists in this
      * PropertyOwner (or any sub-owner). If the identifier contains one or more `.`, the

include/openspace/query/query.h

@@ -42,6 +42,7 @@ Scene* sceneGraph();
 SceneGraphNode* sceneGraphNode(const std::string& name);
 const Renderable* renderable(const std::string& name);
 properties::Property* property(const std::string& uri);
+properties::PropertyOwner* propertyOwner(const std::string& uri);
 std::vector<properties::Property*> allProperties();
 
 } // namespace openspace

modules/globebrowsing/src/layergroup.cpp

@@ -168,20 +168,11 @@ Layer* LayerGroup::addLayer(const ghoul::Dictionary& layerDict) {
     // initialize function. This means that the layerManager does not exists yet, and
     // we cannot find which SGN it belongs to... Want to avoid doing this check, so
     // this should be fixed (probably as part of a cleanup/rewite of the LayerManager)
-    if (!layerManager) {
-        global::eventEngine->publishEvent<events::EventLayerAdded>(
-            "", // we don't know this yet
-            layerGroup->identifier(),
-            ptr->identifier()
-        );
-    }
-    else {
+    if (layerManager) {
         properties::PropertyOwner* globe = layerManager->owner();
         properties::PropertyOwner* sceneGraphNode = globe->owner();
         global::eventEngine->publishEvent<events::EventLayerAdded>(
-            sceneGraphNode->identifier(),
-            layerGroup->identifier(),
-            ptr->identifier()
+            ptr->uri()
         );
     }
 
@@ -203,9 +194,7 @@ void LayerGroup::deleteLayer(const std::string& layerName) {
             properties::PropertyOwner* globe = layerManager->owner();
             properties::PropertyOwner* sceneGraphNode = globe->owner();
             global::eventEngine->publishEvent<events::EventLayerRemoved>(
-                sceneGraphNode->identifier(),
-                layerGroup->identifier(),
-                it->get()->identifier()
+                it->get()->uri()
             );
             // We need to keep the name of the layer since we only get it as a reference
             // and the name needs to survive the deletion

modules/server/src/jsonconverters.cpp

@@ -58,7 +58,8 @@ void to_json(json& j, const PropertyOwner& p) {
         { "description", p.description() },
         { "properties", p.properties() },
         { "subowners", p.propertySubOwners() },
-        { "tag", p.tags() }
+        { "tag", p.tags() },
+        { "uri", p.uri() }
     };
 }
 

modules/server/src/topics/getpropertytopic.cpp

@@ -103,6 +103,10 @@ json GetPropertyTopic::propertyFromKey(const std::string& key) {
     if (prop) {
         return wrappedPayload(prop);
     }
+    properties::PropertyOwner* node = propertyOwner(key);
+    if (node) {
+        return wrappedPayload(node);
+    }
 
     return wrappedError(std::format("Property '{}' not found", key), 404);
 }

scripts/drag_drop_handler.lua

@@ -47,30 +47,28 @@ local is_geojson_file = function(extension)
   return extension == ".geojson"
 end
 
-local ReloadUIScript = [[ if openspace.hasProperty('Modules.CefWebGui.Reload') then openspace.setPropertyValue('Modules.CefWebGui.Reload', nil) end ]]
-
 if is_image_file(extension) then
   return [[
   openspace.addScreenSpaceRenderable({
     Identifier = openspace.makeIdentifier("]] .. basename_without_extension .. [["),
     Type = "ScreenSpaceImageLocal",
     TexturePath = "]] .. filename .. [["
-  });]] .. ReloadUIScript
+  });]] 
 elseif is_video_file(extension) then
   return [[
     openspace.addScreenSpaceRenderable({
       Identifier = openspace.makeIdentifier("]] .. basename_without_extension .. [["),
       Type = "ScreenSpaceVideo",
       Video = "]] .. filename .. [["
-    });]] .. ReloadUIScript
+    });]] 
 elseif is_asset_file(extension) then
   return [[
     if openspace.asset.isLoaded("]] .. filename .. [[") ~= true then
       openspace.printInfo("Adding asset: ']] .. filename .. [[' (drag-and-drop)");
     end
-    openspace.asset.add("]] .. filename .. '");' .. ReloadUIScript
+    openspace.asset.add("]] .. filename .. '");' 
 elseif is_recording_file(extension) then
   return 'openspace.sessionRecording.startPlayback("' .. filename .. '")'
 elseif is_geojson_file(extension) then
-  return 'openspace.globebrowsing.addGeoJsonFromFile("' .. filename .. '")'  .. ReloadUIScript
+  return 'openspace.globebrowsing.addGeoJsonFromFile("' .. filename .. '")'  
 end

src/events/event.cpp

@@ -45,12 +45,12 @@ namespace openspace::events {
 
 void log(int i, const EventSceneGraphNodeAdded& e) {
     ghoul_assert(e.type == EventSceneGraphNodeAdded::Type, "Wrong type");
-    LINFO(std::format("[{}] SceneGraphNodeAdded: {}", i, e.node));
+    LINFO(std::format("[{}] SceneGraphNodeAdded: {}", i, e.uri));
 }
 
 void log(int i, const EventSceneGraphNodeRemoved& e) {
     ghoul_assert(e.type == EventSceneGraphNodeRemoved::Type, "Wrong type");
-    LINFO(std::format("[{}] SceneGraphNodeRemoved: {}", i, e.node));
+    LINFO(std::format("[{}] SceneGraphNodeRemoved: {}", i, e.uri));
 }
 
 void log(int i, const EventParallelConnection& e) {
@@ -87,12 +87,12 @@ void log(int i, const EventApplicationShutdown& e) {
 
 void log(int i, const EventScreenSpaceRenderableAdded& e) {
     ghoul_assert(e.type == EventScreenSpaceRenderableAdded::Type, "Wrong type");
-    LINFO(std::format("[{}] ScreenSpaceRenderableAdded: {}", i, e.renderable));
+    LINFO(std::format("[{}] ScreenSpaceRenderableAdded: {}", i, e.uri));
 }
 
 void log(int i, const EventScreenSpaceRenderableRemoved& e) {
     ghoul_assert(e.type == EventScreenSpaceRenderableRemoved::Type, "Wrong type");
-    LINFO(std::format("[{}] ScreenSpaceRenderableRemoved: {}", i, e.renderable));
+    LINFO(std::format("[{}] ScreenSpaceRenderableRemoved: {}", i, e.uri));
 }
 
 void log(int i, const EventCameraFocusTransition& e) {
@@ -148,12 +148,12 @@ void log(int i, const EventFocusNodeChanged& e) {
 
 void log(int i, const EventLayerAdded& e) {
     ghoul_assert(e.type == EventLayerAdded::Type, "Wrong type");
-    LINFO(std::format("[{}] LayerAdded: {}", i, e.layer));
+    LINFO(std::format("[{}] LayerAdded: {}", i, e.uri));
 }
 
 void log(int i, const EventLayerRemoved& e) {
     ghoul_assert(e.type == EventLayerRemoved::Type, "Wrong type");
-    LINFO(std::format("[{}] LayerRemoved: {}", i, e.layer));
+    LINFO(std::format("[{}] LayerRemoved: {}", i, e.uri));
 }
 
 void log(int i, const EventSessionRecordingPlayback& e) {
@@ -335,14 +335,14 @@ ghoul::Dictionary toParameter(const Event& e) {
     switch (e.type) {
         case Event::Type::SceneGraphNodeAdded:
             d.setValue(
-                "Node",
-                std::string(static_cast<const EventSceneGraphNodeAdded&>(e).node)
+                "Uri",
+                std::string(static_cast<const EventSceneGraphNodeAdded&>(e).uri)
             );
             break;
         case Event::Type::SceneGraphNodeRemoved:
             d.setValue(
-                "Node",
-                std::string(static_cast<const EventSceneGraphNodeRemoved&>(e).node)
+                "Uri",
+                std::string(static_cast<const EventSceneGraphNodeRemoved&>(e).uri)
             );
             break;
         case Event::Type::ParallelConnection:
@@ -376,17 +376,17 @@ ghoul::Dictionary toParameter(const Event& e) {
             break;
         case Event::Type::ScreenSpaceRenderableAdded:
             d.setValue(
-                "Renderable",
+                "Uri",
                 std::string(
-                    static_cast<const EventScreenSpaceRenderableAdded&>(e).renderable
+                    static_cast<const EventScreenSpaceRenderableAdded&>(e).uri
                 )
             );
             break;
         case Event::Type::ScreenSpaceRenderableRemoved:
             d.setValue(
-                "Renderable",
+                "Uri",
                 std::string(
-                    static_cast<const EventScreenSpaceRenderableRemoved&>(e).renderable
+                    static_cast<const EventScreenSpaceRenderableRemoved&>(e).uri
                 )
             );
             break;
@@ -438,30 +438,14 @@ ghoul::Dictionary toParameter(const Event& e) {
             break;
         case Event::Type::LayerAdded:
             d.setValue(
-                "Globe",
-                std::string(static_cast<const EventLayerAdded&>(e).node)
-            );
-            d.setValue(
-                "Group",
-                std::string(static_cast<const EventLayerAdded&>(e).layerGroup)
-            );
-            d.setValue(
-                "Layer",
-                std::string(static_cast<const EventLayerAdded&>(e).layer)
+                "Uri",
+                std::string(static_cast<const EventLayerAdded&>(e).uri)
             );
             break;
         case Event::Type::LayerRemoved:
             d.setValue(
-                "Globe",
-                std::string(static_cast<const EventLayerRemoved&>(e).node)
-            );
-            d.setValue(
-                "Group",
-                std::string(static_cast<const EventLayerRemoved&>(e).layerGroup)
-            );
-            d.setValue(
-                "Layer",
-                std::string(static_cast<const EventLayerRemoved&>(e).layer)
+                "Uri",
+                std::string(static_cast<const EventLayerRemoved&>(e).uri)
             );
             break;
         case Event::Type::SessionRecordingPlayback:
@@ -624,12 +608,12 @@ void logAllEvents(const Event* e) {
 
 EventSceneGraphNodeAdded::EventSceneGraphNodeAdded(const SceneGraphNode* node_)
     : Event(Type)
-    , node(temporaryString(node_->identifier()))
+    , uri(temporaryString(node_->uri()))
 {}
 
 EventSceneGraphNodeRemoved::EventSceneGraphNodeRemoved(const SceneGraphNode* node_)
     : Event(Type)
-    , node(temporaryString(node_->identifier()))
+    , uri(temporaryString(node_->uri()))
 {}
 
 EventParallelConnection::EventParallelConnection(State state_)
@@ -649,13 +633,13 @@ EventApplicationShutdown::EventApplicationShutdown(State state_)
 EventScreenSpaceRenderableAdded::EventScreenSpaceRenderableAdded(
                                                  const ScreenSpaceRenderable* renderable_)
     : Event(Type)
-    , renderable(temporaryString(renderable_->identifier()))
+    , uri(temporaryString(renderable_->uri()))
 {}
 
 EventScreenSpaceRenderableRemoved::EventScreenSpaceRenderableRemoved(
                                                  const ScreenSpaceRenderable* renderable_)
     : Event(Type)
-    , renderable(temporaryString(renderable_->identifier()))
+    , uri(temporaryString(renderable_->uri()))
 {}
 
 EventCameraFocusTransition::EventCameraFocusTransition(const Camera* camera_,
@@ -700,20 +684,14 @@ EventFocusNodeChanged::EventFocusNodeChanged(const SceneGraphNode* oldNode_,
     ghoul_assert(newNode_, "There must be a new node");
 }
 
-EventLayerAdded::EventLayerAdded(std::string_view node_, std::string_view layerGroup_,
-                                 std::string_view layer_)
+EventLayerAdded::EventLayerAdded(std::string_view uri_)
     : Event(Type)
-    , node(temporaryString(node_))
-    , layerGroup(temporaryString(layerGroup_))
-    , layer(temporaryString(layer_))
+    , uri(temporaryString(uri_))
 {}
 
-EventLayerRemoved::EventLayerRemoved(std::string_view node_, std::string_view layerGroup_,
-                                     std::string_view layer_)
+EventLayerRemoved::EventLayerRemoved(std::string_view uri_)
     : Event(Type)
-    , node(temporaryString(node_))
-    , layerGroup(temporaryString(layerGroup_))
-    , layer(temporaryString(layer_))
+    , uri(temporaryString(uri_))
 {}
 
 EventSessionRecordingPlayback::EventSessionRecordingPlayback(State state_)

src/properties/property.cpp

@@ -78,13 +78,9 @@ const std::string& Property::identifier() const {
 
 std::string Property::fullyQualifiedIdentifier() const {
     std::string identifier = _identifier;
-    PropertyOwner* currentOwner = owner();
-    while (currentOwner) {
-        const std::string& ownerId = currentOwner->identifier();
-        if (!ownerId.empty()) {
-            identifier = std::format("{}.{}", ownerId, identifier);
-        }
-        currentOwner = currentOwner->owner();
+    const std::string& ownerUri = owner()->uri();
+    if (!ownerUri.empty()) {
+        identifier = std::format("{}.{}", ownerUri, identifier);
     }
     return identifier;
 }

src/properties/propertyowner.cpp

@@ -111,6 +111,41 @@ Property* PropertyOwner::property(const std::string& uri) const {
     }
 }
 
+PropertyOwner* PropertyOwner::propertyOwner(const std::string& uri) const {
+    PropertyOwner* directChild = propertySubOwner(uri);
+    if (directChild) {
+        return directChild;
+    }
+
+    // If we do not own the searched PropertyOwner, it must consist of a concatenated
+    // name and we can delegate it to a subowner
+    const size_t ownerSeparator = uri.find(URISeparator);
+    if (ownerSeparator == std::string::npos) {
+        // if we do not own the PropertyOwner and there is no separator, it does not exist
+        return nullptr;
+    }
+    else {
+        const std::string parentName = uri.substr(0, ownerSeparator);
+        const std::string ownerName = uri.substr(ownerSeparator + 1);
+
+        PropertyOwner* owner = propertySubOwner(parentName);
+        return owner ? owner->propertyOwner(ownerName) : nullptr;
+    }
+}
+
+std::string PropertyOwner::uri() const {
+    std::string identifier = _identifier;
+    PropertyOwner* currentOwner = owner();
+    while (currentOwner) {
+        const std::string& ownerId = currentOwner->identifier();
+        if (!ownerId.empty()) {
+            identifier = std::format("{}.{}", ownerId, identifier);
+        }
+        currentOwner = currentOwner->owner();
+    }
+    return identifier;
+}
+
 bool PropertyOwner::hasProperty(const std::string& uri) const {
     return property(uri) != nullptr;
 }

src/query/query.cpp

@@ -55,6 +55,11 @@ properties::Property* property(const std::string& uri) {
     return property;
 }
 
+properties::PropertyOwner* propertyOwner(const std::string& uri) {
+    properties::PropertyOwner* property = global::rootPropertyOwner->propertyOwner(uri);
+    return property;
+}
+
 std::vector<properties::Property*> allProperties() {
     return global::rootPropertyOwner->propertiesRecursive();
 }