[librsvg: 16/22] Make get_node() return a Result, not an Option

[Federico Mena Quintero <federico src gnome org>]

commit abb4e684b616b8f11c6764935d0e76ac603361be
Author: Federico Mena Quintero <federico gnome org>
Date:   Wed Oct 2 09:30:22 2019 -0500

    Make get_node() return a Result, not an Option

 rsvg_internals/src/drawing_ctx.rs   | 32 +++++++++++++++++++++-----------
 rsvg_internals/src/filters/image.rs |  2 +-
 rsvg_internals/src/gradient.rs      |  1 -
 rsvg_internals/src/structure.rs     |  4 ++--
 rsvg_internals/src/text.rs          |  2 +-
 5 files changed, 25 insertions(+), 16 deletions(-)
---
diff --git a/rsvg_internals/src/drawing_ctx.rs b/rsvg_internals/src/drawing_ctx.rs
index 69574cad..7e76ba06 100644
--- a/rsvg_internals/src/drawing_ctx.rs
+++ b/rsvg_internals/src/drawing_ctx.rs
@@ -616,7 +616,7 @@ impl DrawingCtx {
             } => {
                 let mut had_paint_server = false;
 
-                if let Some(acquired) = self.acquired_nodes.get_node(iri) {
+                if let Ok(acquired) = self.acquired_nodes.get_node(iri) {
                     let node = acquired.get();
 
                     had_paint_server = match node.borrow().get_type() {
@@ -1095,16 +1095,27 @@ impl AcquiredNodes {
     // Note that if you acquire a node, you have to release it before trying to
     // acquire it again.  If you acquire a node "#foo" and don't release it before
     // trying to acquire "foo" again, you will obtain a %NULL the second time.
-    pub fn get_node(&self, fragment: &Fragment) -> Option<AcquiredNode> {
-        if let Ok(node) = self.svg.lookup(fragment) {
-            if !self.node_stack.borrow().contains(&node) {
-                self.node_stack.borrow_mut().push(&node);
-                let acq = AcquiredNode(self.node_stack.clone(), node.clone());
-                return Some(acq);
-            }
+    pub fn get_node(&self, fragment: &Fragment) -> Result<AcquiredNode, AcquireError> {
+        let node = self.svg.lookup(fragment)
+            .map_err(|_| {
+                // FIXME: callers shouldn't have to know that get_node() can initiate a file load.
+                // Maybe we should have the following stages:
+                //   - load main SVG XML
+                //
+                //   - load secondary SVG XML and other files like images;
+                //     all svg::Resources and svg::Images loaded
+                //
+                //   - Now that all files are loaded, resolve URL references
+                AcquireError::LinkNotFound(fragment.clone())
+            })?;
+
+        if self.node_stack.borrow().contains(&node) {
+            Err(AcquireError::CircularReference(fragment.clone()))
+        } else {
+            self.node_stack.borrow_mut().push(&node);
+            let acquired = AcquiredNode(self.node_stack.clone(), node.clone());
+            Ok(acquired)
         }
-
-        None
     }
 
     // Use this function when looking up urls to other nodes, and when you expect
@@ -1123,7 +1134,6 @@ impl AcquiredNodes {
         node_type: NodeType,
     ) -> Result<AcquiredNode, AcquireError> {
         self.get_node(fragment)
-            .ok_or_else(|| AcquireError::LinkNotFound(fragment.clone()))
             .and_then(|acquired| {
                 if acquired.get().borrow().get_type() == node_type {
                     Ok(acquired)
diff --git a/rsvg_internals/src/filters/image.rs b/rsvg_internals/src/filters/image.rs
index c0ca11c2..98e064e7 100644
--- a/rsvg_internals/src/filters/image.rs
+++ b/rsvg_internals/src/filters/image.rs
@@ -47,7 +47,7 @@ impl Image {
         let acquired_drawable = draw_ctx
             .acquired_nodes()
             .get_node(fragment)
-            .ok_or(FilterError::InvalidInput)?;
+            .map_err(|_| FilterError::InvalidInput)?;
         let drawable = acquired_drawable.get();
 
         let surface = ImageSurface::create(
diff --git a/rsvg_internals/src/gradient.rs b/rsvg_internals/src/gradient.rs
index 3aa0b929..ee386456 100644
--- a/rsvg_internals/src/gradient.rs
+++ b/rsvg_internals/src/gradient.rs
@@ -782,7 +782,6 @@ fn acquire_gradient<'a>(
     fragment: &Fragment,
 ) -> Result<AcquiredNode, AcquireError> {
     draw_ctx.acquired_nodes().get_node(fragment)
-        .ok_or(AcquireError::LinkNotFound(fragment.clone()))
         .and_then(|acquired| {
             let node_type = acquired.get().borrow().get_type();
 
diff --git a/rsvg_internals/src/structure.rs b/rsvg_internals/src/structure.rs
index c9da97e7..62489928 100644
--- a/rsvg_internals/src/structure.rs
+++ b/rsvg_internals/src/structure.rs
@@ -315,13 +315,13 @@ impl NodeTrait for NodeUse {
 
         let link = self.link.as_ref().unwrap();
 
-        let child = if let Some(acquired) = draw_ctx.acquired_nodes().get_node(link) {
+        let child = if let Ok(acquired) = draw_ctx.acquired_nodes().get_node(link) {
             // Here we clone the acquired child, so that we can drop the AcquiredNode as
             // early as possible.  This is so that the child's drawing method will be able
             // to re-acquire the child for other purposes.
             acquired.get().clone()
         } else {
-            rsvg_log!("element {} references nonexistent \"{}\"", node, link,);
+            rsvg_log!("element {} references nonexistent \"{}\"", node, link);
             return Ok(draw_ctx.empty_bbox());
         };
 
diff --git a/rsvg_internals/src/text.rs b/rsvg_internals/src/text.rs
index 8de8424b..525cdb5f 100644
--- a/rsvg_internals/src/text.rs
+++ b/rsvg_internals/src/text.rs
@@ -676,7 +676,7 @@ impl NodeTRef {
         let link = self.link.as_ref().unwrap();
         let values = cascaded.get();
 
-        if let Some(acquired) = draw_ctx.acquired_nodes().get_node(link) {
+        if let Ok(acquired) = draw_ctx.acquired_nodes().get_node(link) {
             let c = acquired.get();
             extract_chars_children_to_chunks_recursively(chunks, &c, values, depth);
         } else {