[librsvg: 2/5] Remove defs.rs and move to the relevant files



commit 5f0ba352513ca7929acc3c2fe6ccfe304f6b4910
Author: Paolo Borelli <pborelli gnome org>
Date:   Fri Jan 11 21:13:26 2019 +0100

    Remove defs.rs and move to the relevant files
    
    The Defs struct goes into svg.rs, where it can be private and it is
    renamed to Resources, since Defs is confusing (it has nothing to
    do with NodeDefs anymore, since now we are storing ids in Svg itself).
    The Href parsing code goes into allowed_url.rs.
    Errors go to error.rs.

 Makefile.am                         |   1 -
 po/POTFILES.in                      |   1 -
 rsvg_internals/src/allowed_url.rs   | 174 +++++++++++++++++++++++
 rsvg_internals/src/defs.rs          | 276 ------------------------------------
 rsvg_internals/src/drawing_ctx.rs   |   2 +-
 rsvg_internals/src/error.rs         |  33 ++++-
 rsvg_internals/src/filters/image.rs |   2 +-
 rsvg_internals/src/gradient.rs      |   2 +-
 rsvg_internals/src/handle.rs        |   3 +-
 rsvg_internals/src/image.rs         |   2 +-
 rsvg_internals/src/iri.rs           |   2 +-
 rsvg_internals/src/lib.rs           |   1 -
 rsvg_internals/src/marker.rs        |   2 +-
 rsvg_internals/src/paint_server.rs  |   2 +-
 rsvg_internals/src/pattern.rs       |   2 +-
 rsvg_internals/src/structure.rs     |   2 +-
 rsvg_internals/src/svg.rs           |  68 ++++++++-
 rsvg_internals/src/text.rs          |   2 +-
 18 files changed, 278 insertions(+), 299 deletions(-)
---
diff --git a/Makefile.am b/Makefile.am
index ce2fe885..392b46bf 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -50,7 +50,6 @@ RUST_SRC =                                                    \
        rsvg_internals/src/create_node.rs                       \
        rsvg_internals/src/croco.rs                             \
        rsvg_internals/src/css.rs                               \
-       rsvg_internals/src/defs.rs                              \
        rsvg_internals/src/dpi.rs                               \
        rsvg_internals/src/drawing_ctx.rs                       \
        rsvg_internals/src/filters/bounds.rs                    \
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 2939779d..2365aee6 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -25,7 +25,6 @@ rsvg_internals/src/coord_units.rs
 rsvg_internals/src/create_node.rs
 rsvg_internals/src/croco.rs
 rsvg_internals/src/css.rs
-rsvg_internals/src/defs.rs
 rsvg_internals/src/drawing_ctx.rs
 rsvg_internals/src/error.rs
 rsvg_internals/src/filters/blend.rs
diff --git a/rsvg_internals/src/allowed_url.rs b/rsvg_internals/src/allowed_url.rs
index bb2e400c..525185d4 100644
--- a/rsvg_internals/src/allowed_url.rs
+++ b/rsvg_internals/src/allowed_url.rs
@@ -4,6 +4,8 @@ use std::io;
 use std::path::{Path, PathBuf};
 use url::{self, Url};
 
+use error::HrefError;
+
 /// Wrapper for URLs which are allowed to be loaded
 ///
 /// SVG files can reference other files (PNG/JPEG images, other SVGs,
@@ -151,6 +153,107 @@ fn canonicalize<P: AsRef<Path>>(path: P) -> Result<PathBuf, io::Error> {
     Ok(path.as_ref().to_path_buf())
 }
 
+/// Parsed result of an href from an SVG or CSS file
+///
+/// Sometimes in SVG element references (e.g. the `href` in the `<feImage>` element) we
+/// must decide between referencing an external file, or using a plain fragment identifier
+/// like `href="#foo"` as a reference to an SVG element in the same file as the one being
+/// processed.  This enum makes that distinction.
+#[derive(Debug, PartialEq)]
+pub enum Href {
+    PlainUri(String),
+    WithFragment(Fragment),
+}
+
+/// Optional URI, mandatory fragment id
+#[derive(Debug, PartialEq, Clone)]
+pub struct Fragment(Option<String>, String);
+
+impl Fragment {
+    // Outside of testing, we don't want code creating Fragments by hand;
+    // they should get them from Href.
+    #[cfg(test)]
+    pub fn new(uri: Option<String>, fragment: String) -> Fragment {
+        Fragment(uri, fragment)
+    }
+
+    pub fn parse(href: &str) -> Result<Fragment, HrefError> {
+        let href = Href::with_fragment(href)?;
+
+        if let Href::WithFragment(f) = href {
+            Ok(f)
+        } else {
+            unreachable!();
+        }
+    }
+
+    pub fn uri(&self) -> Option<&str> {
+        self.0.as_ref().map(|s| s.as_str())
+    }
+
+    pub fn fragment(&self) -> &str {
+        &self.1
+    }
+}
+
+impl fmt::Display for Fragment {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(
+            f,
+            "{}#{}",
+            self.0.as_ref().map(String::as_str).unwrap_or(""),
+            self.1
+        )
+    }
+}
+
+impl Href {
+    /// Parses a string into an Href, or returns an error
+    ///
+    /// An href can come from an `xlink:href` attribute in an SVG
+    /// element.  This function determines if the provided href is a
+    /// plain absolute or relative URL ("`foo.png`"), or one with a
+    /// fragment identifier ("`foo.svg#bar`").
+    pub fn parse(href: &str) -> Result<Href, HrefError> {
+        let (uri, fragment) = match href.rfind('#') {
+            None => (Some(href), None),
+            Some(p) if p == 0 => (None, Some(&href[1..])),
+            Some(p) => (Some(&href[..p]), Some(&href[(p + 1)..])),
+        };
+
+        match (uri, fragment) {
+            (None, Some(f)) if f.len() == 0 => Err(HrefError::ParseError),
+            (None, Some(f)) => Ok(Href::WithFragment(Fragment(None, f.to_string()))),
+            (Some(u), _) if u.len() == 0 => Err(HrefError::ParseError),
+            (Some(u), None) => Ok(Href::PlainUri(u.to_string())),
+            (Some(_u), Some(f)) if f.len() == 0 => Err(HrefError::ParseError),
+            (Some(u), Some(f)) => Ok(Href::WithFragment(Fragment(
+                Some(u.to_string()),
+                f.to_string(),
+            ))),
+            (_, _) => Err(HrefError::ParseError),
+        }
+    }
+
+    pub fn without_fragment(href: &str) -> Result<Href, HrefError> {
+        use self::Href::*;
+
+        match Href::parse(href)? {
+            r @ PlainUri(_) => Ok(r),
+            WithFragment(_) => Err(HrefError::FragmentForbidden),
+        }
+    }
+
+    pub fn with_fragment(href: &str) -> Result<Href, HrefError> {
+        use self::Href::*;
+
+        match Href::parse(href)? {
+            PlainUri(_) => Err(HrefError::FragmentRequired),
+            r @ WithFragment(_) => Ok(r),
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -254,4 +357,75 @@ mod tests {
             Err(AllowedUrlError::NotSiblingOrChildOfBaseFile)
         );
     }
+
+    #[test]
+    fn parses_href() {
+        assert_eq!(
+            Href::parse("uri").unwrap(),
+            Href::PlainUri("uri".to_string())
+        );
+        assert_eq!(
+            Href::parse("#fragment").unwrap(),
+            Href::WithFragment(Fragment::new(None, "fragment".to_string()))
+        );
+        assert_eq!(
+            Href::parse("uri#fragment").unwrap(),
+            Href::WithFragment(Fragment::new(
+                Some("uri".to_string()),
+                "fragment".to_string()
+            ))
+        );
+
+        assert_eq!(Href::parse(""), Err(HrefError::ParseError));
+        assert_eq!(Href::parse("#"), Err(HrefError::ParseError));
+        assert_eq!(Href::parse("uri#"), Err(HrefError::ParseError));
+    }
+
+    #[test]
+    fn href_without_fragment() {
+        assert_eq!(
+            Href::without_fragment("uri").unwrap(),
+            Href::PlainUri("uri".to_string())
+        );
+
+        assert_eq!(
+            Href::without_fragment("#foo"),
+            Err(HrefError::FragmentForbidden)
+        );
+
+        assert_eq!(
+            Href::without_fragment("uri#foo"),
+            Err(HrefError::FragmentForbidden)
+        );
+    }
+
+    #[test]
+    fn href_with_fragment() {
+        assert_eq!(
+            Href::with_fragment("#foo").unwrap(),
+            Href::WithFragment(Fragment::new(None, "foo".to_string()))
+        );
+
+        assert_eq!(
+            Href::with_fragment("uri#foo").unwrap(),
+            Href::WithFragment(Fragment::new(Some("uri".to_string()), "foo".to_string()))
+        );
+
+        assert_eq!(Href::with_fragment("uri"), Err(HrefError::FragmentRequired));
+    }
+
+    #[test]
+    fn parses_fragment() {
+        assert_eq!(
+            Fragment::parse("#foo").unwrap(),
+            Fragment::new(None, "foo".to_string())
+        );
+
+        assert_eq!(
+            Fragment::parse("uri#foo").unwrap(),
+            Fragment::new(Some("uri".to_string()), "foo".to_string())
+        );
+
+        assert_eq!(Fragment::parse("uri"), Err(HrefError::FragmentRequired));
+    }
 }
diff --git a/rsvg_internals/src/drawing_ctx.rs b/rsvg_internals/src/drawing_ctx.rs
index 9fded6c2..6496e418 100644
--- a/rsvg_internals/src/drawing_ctx.rs
+++ b/rsvg_internals/src/drawing_ctx.rs
@@ -5,10 +5,10 @@ use glib::translate::*;
 use std::cell::RefCell;
 use std::rc::{Rc, Weak};
 
+use allowed_url::Fragment;
 use bbox::BoundingBox;
 use clip_path::{ClipPathUnits, NodeClipPath};
 use coord_units::CoordUnits;
-use defs::Fragment;
 use dpi::Dpi;
 use error::RenderingError;
 use filters;
diff --git a/rsvg_internals/src/error.rs b/rsvg_internals/src/error.rs
index 58537172..b30360f0 100644
--- a/rsvg_internals/src/error.rs
+++ b/rsvg_internals/src/error.rs
@@ -10,7 +10,6 @@ use glib_sys;
 use libc;
 
 use attributes::Attribute;
-use defs::HrefError;
 use parsers::ParseError;
 
 /// A simple error which refers to an attribute's value
@@ -147,6 +146,38 @@ impl<O, E: Into<ValueErrorKind>> AttributeResultExt<O, E> for Result<O, E> {
     }
 }
 
+/// Errors returned when creating an `Href` out of a string
+#[derive(Debug, Clone, PartialEq)]
+pub enum HrefError {
+    /// The href is an invalid URI or has empty components.
+    ParseError,
+
+    /// A fragment identifier ("`#foo`") is not allowed here
+    ///
+    /// For example, the SVG `<image>` element only allows referencing
+    /// resources without fragment identifiers like
+    /// `xlink:href="foo.png"`.
+    FragmentForbidden,
+
+    /// A fragment identifier ("`#foo`") was required but not found.  For example,
+    /// the SVG `<use>` element requires one, as in `<use xlink:href="foo.svg#bar">`.
+    FragmentRequired,
+}
+
+impl From<HrefError> for ValueErrorKind {
+    fn from(e: HrefError) -> ValueErrorKind {
+        match e {
+            HrefError::ParseError => ValueErrorKind::Parse(ParseError::new("url parse error")),
+            HrefError::FragmentForbidden => {
+                ValueErrorKind::Value("fragment identifier not allowed".to_string())
+            }
+            HrefError::FragmentRequired => {
+                ValueErrorKind::Value("fragment identifier required".to_string())
+            }
+        }
+    }
+}
+
 #[derive(Debug, Clone)]
 pub enum LoadingError {
     NoDataPassedToParser,
diff --git a/rsvg_internals/src/filters/image.rs b/rsvg_internals/src/filters/image.rs
index ee4f79a7..f42a1cb0 100644
--- a/rsvg_internals/src/filters/image.rs
+++ b/rsvg_internals/src/filters/image.rs
@@ -2,9 +2,9 @@ use std::cell::{Cell, RefCell};
 
 use cairo::{self, ImageSurface, MatrixTrait, PatternTrait};
 
+use allowed_url::{Fragment, Href};
 use aspect_ratio::AspectRatio;
 use attributes::Attribute;
-use defs::{Fragment, Href};
 use drawing_ctx::DrawingCtx;
 use error::{NodeError, RenderingError};
 use handle::{self, LoadOptions};
diff --git a/rsvg_internals/src/gradient.rs b/rsvg_internals/src/gradient.rs
index b79179f4..507ffac2 100644
--- a/rsvg_internals/src/gradient.rs
+++ b/rsvg_internals/src/gradient.rs
@@ -3,10 +3,10 @@ use cssparser::{self, CowRcStr, Parser, Token};
 
 use std::cell::RefCell;
 
+use allowed_url::Fragment;
 use attributes::Attribute;
 use bbox::*;
 use coord_units::CoordUnits;
-use defs::Fragment;
 use drawing_ctx::{AcquiredNode, DrawingCtx, NodeStack};
 use error::*;
 use length::*;
diff --git a/rsvg_internals/src/handle.rs b/rsvg_internals/src/handle.rs
index e0adf6ee..bb29511b 100644
--- a/rsvg_internals/src/handle.rs
+++ b/rsvg_internals/src/handle.rs
@@ -19,8 +19,7 @@ use gobject_sys;
 use libc;
 use url::Url;
 
-use allowed_url::AllowedUrl;
-use defs::Href;
+use allowed_url::{AllowedUrl, Href};
 use dpi::Dpi;
 use drawing_ctx::{DrawingCtx, RsvgRectangle};
 use error::{set_gerror, DefsLookupErrorKind, LoadingError, RenderingError};
diff --git a/rsvg_internals/src/image.rs b/rsvg_internals/src/image.rs
index aa920667..6aa5802f 100644
--- a/rsvg_internals/src/image.rs
+++ b/rsvg_internals/src/image.rs
@@ -2,10 +2,10 @@ use cairo;
 use cairo::{MatrixTrait, PatternTrait};
 use std::cell::{Cell, RefCell};
 
+use allowed_url::Href;
 use aspect_ratio::AspectRatio;
 use attributes::Attribute;
 use bbox::BoundingBox;
-use defs::Href;
 use drawing_ctx::DrawingCtx;
 use error::{NodeError, RenderingError};
 use float_eq_cairo::ApproxEqCairo;
diff --git a/rsvg_internals/src/iri.rs b/rsvg_internals/src/iri.rs
index ca0280bf..35f7d884 100644
--- a/rsvg_internals/src/iri.rs
+++ b/rsvg_internals/src/iri.rs
@@ -1,6 +1,6 @@
 use cssparser::Parser;
 
-use defs::{Fragment, Href};
+use allowed_url::{Fragment, Href};
 use parsers::Parse;
 use parsers::ParseError;
 
diff --git a/rsvg_internals/src/lib.rs b/rsvg_internals/src/lib.rs
index c1c2ff44..fc3f6dbb 100644
--- a/rsvg_internals/src/lib.rs
+++ b/rsvg_internals/src/lib.rs
@@ -94,7 +94,6 @@ mod cond;
 mod create_node;
 mod croco;
 mod css;
-mod defs;
 mod dpi;
 mod drawing_ctx;
 mod error;
diff --git a/rsvg_internals/src/marker.rs b/rsvg_internals/src/marker.rs
index e7c054dd..4a291da2 100644
--- a/rsvg_internals/src/marker.rs
+++ b/rsvg_internals/src/marker.rs
@@ -5,10 +5,10 @@ use std::ops::Deref;
 use cairo::MatrixTrait;
 use cssparser::{CowRcStr, Parser, Token};
 
+use allowed_url::Fragment;
 use angle::Angle;
 use aspect_ratio::*;
 use attributes::Attribute;
-use defs::Fragment;
 use drawing_ctx::DrawingCtx;
 use error::*;
 use float_eq_cairo::ApproxEqCairo;
diff --git a/rsvg_internals/src/paint_server.rs b/rsvg_internals/src/paint_server.rs
index 48c4183a..ebef8d77 100644
--- a/rsvg_internals/src/paint_server.rs
+++ b/rsvg_internals/src/paint_server.rs
@@ -1,7 +1,7 @@
 use cssparser::{self, Parser};
 
+use allowed_url::Fragment;
 use bbox::BoundingBox;
-use defs::Fragment;
 use drawing_ctx::DrawingCtx;
 use error::*;
 use node::RsvgNode;
diff --git a/rsvg_internals/src/pattern.rs b/rsvg_internals/src/pattern.rs
index 892fac06..729db60c 100644
--- a/rsvg_internals/src/pattern.rs
+++ b/rsvg_internals/src/pattern.rs
@@ -5,11 +5,11 @@ use std::cell::RefCell;
 use std::f64;
 use std::rc::*;
 
+use allowed_url::Fragment;
 use aspect_ratio::*;
 use attributes::Attribute;
 use bbox::*;
 use coord_units::CoordUnits;
-use defs::Fragment;
 use drawing_ctx::{DrawingCtx, NodeStack};
 use error::{AttributeResultExt, RenderingError};
 use float_eq_cairo::ApproxEqCairo;
diff --git a/rsvg_internals/src/structure.rs b/rsvg_internals/src/structure.rs
index 57c772f8..a50ead3f 100644
--- a/rsvg_internals/src/structure.rs
+++ b/rsvg_internals/src/structure.rs
@@ -1,10 +1,10 @@
 use std::cell::Cell;
 use std::cell::RefCell;
 
+use allowed_url::Fragment;
 use aspect_ratio::*;
 use attributes::Attribute;
 use css::CssStyles;
-use defs::Fragment;
 use dpi::Dpi;
 use drawing_ctx::DrawingCtx;
 use error::{AttributeResultExt, RenderingError};
diff --git a/rsvg_internals/src/svg.rs b/rsvg_internals/src/svg.rs
index 8dd32ce0..8c60bd7e 100644
--- a/rsvg_internals/src/svg.rs
+++ b/rsvg_internals/src/svg.rs
@@ -1,10 +1,12 @@
 use std::cell::RefCell;
+use std::collections::hash_map::Entry;
 use std::collections::HashMap;
 
 use gio;
+use gobject_sys;
 
+use allowed_url::{AllowedUrl, Fragment};
 use css::CssStyles;
-use defs::{Defs, Fragment};
 use error::LoadingError;
 use handle::{self, LoadOptions, RsvgHandle};
 use node::RsvgNode;
@@ -21,12 +23,12 @@ pub struct Svg {
 
     pub tree: Tree,
 
+    ids: HashMap<String, RsvgNode>,
+
     // This requires interior mutability because we load the extern
-    // defs all over the place.  Eventually we'll be able to do this
+    // resources all over the place.  Eventually we'll be able to do this
     // once, at loading time, and keep this immutable.
-    pub defs: RefCell<Defs>,
-
-    ids: HashMap<String, RsvgNode>,
+    externs: RefCell<Resources>,
 
     pub css_styles: CssStyles,
 }
@@ -41,7 +43,7 @@ impl Svg {
         Svg {
             handle,
             tree,
-            defs: RefCell::new(Defs::new()),
+            externs: RefCell::new(Resources::new()),
             ids,
             css_styles,
         }
@@ -69,7 +71,7 @@ impl Svg {
 
     pub fn lookup(&self, fragment: &Fragment) -> Option<RsvgNode> {
         if fragment.uri().is_some() {
-            self.defs
+            self.externs
                 .borrow_mut()
                 .lookup(&handle::get_load_options(self.handle), fragment)
         } else {
@@ -81,3 +83,55 @@ impl Svg {
         self.ids.get(id).map(|n| (*n).clone())
     }
 }
+
+struct Resources {
+    resources: HashMap<AllowedUrl, *mut RsvgHandle>,
+}
+
+impl Resources {
+    pub fn new() -> Resources {
+        Resources {
+            resources: Default::default(),
+        }
+    }
+
+    /// Returns a node referenced by a fragment ID, from an
+    /// externally-loaded SVG file.
+    pub fn lookup(&mut self, load_options: &LoadOptions, fragment: &Fragment) -> Option<RsvgNode> {
+        if let Some(ref href) = fragment.uri() {
+            match self.get_extern_handle(load_options, href) {
+                Ok(extern_handle) => handle::lookup_fragment_id(extern_handle, fragment.fragment()),
+                Err(()) => None,
+            }
+        } else {
+            unreachable!();
+        }
+    }
+
+    fn get_extern_handle(
+        &mut self,
+        load_options: &LoadOptions,
+        href: &str,
+    ) -> Result<*const RsvgHandle, ()> {
+        let aurl = AllowedUrl::from_href(href, load_options.base_url.as_ref()).map_err(|_| ())?;
+
+        match self.resources.entry(aurl) {
+            Entry::Occupied(e) => Ok(*(e.get())),
+            Entry::Vacant(e) => {
+                let extern_handle = handle::load_extern(load_options, e.key())?;
+                e.insert(extern_handle);
+                Ok(extern_handle)
+            }
+        }
+    }
+}
+
+impl Drop for Resources {
+    fn drop(&mut self) {
+        for (_, handle) in self.resources.iter() {
+            unsafe {
+                gobject_sys::g_object_unref(*handle as *mut _);
+            }
+        }
+    }
+}
diff --git a/rsvg_internals/src/text.rs b/rsvg_internals/src/text.rs
index 436cda91..21884f22 100644
--- a/rsvg_internals/src/text.rs
+++ b/rsvg_internals/src/text.rs
@@ -4,9 +4,9 @@ use pango_sys;
 use pangocairo;
 use std::cell::{Cell, RefCell};
 
+use allowed_url::Fragment;
 use attributes::Attribute;
 use bbox::BoundingBox;
-use defs::Fragment;
 use drawing_ctx::DrawingCtx;
 use error::{AttributeResultExt, RenderingError};
 use float_eq_cairo::ApproxEqCairo;


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]