From 3ffd851d5961b359495200329154cf5ddf225f97 Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Thu, 29 Jul 2021 17:54:23 -0400 Subject: [PATCH 1/4] Add method to get XML namespaces defined directly on a given node --- spec/std/xml/xml_spec.cr | 181 ++++++++++++++++++++++++++++++++++----- src/xml/node.cr | 23 +++-- 2 files changed, 175 insertions(+), 29 deletions(-) diff --git a/spec/std/xml/xml_spec.cr b/spec/std/xml/xml_spec.cr index f1c3e684f8e4..fe5b410a18e6 100644 --- a/spec/std/xml/xml_spec.cr +++ b/spec/std/xml/xml_spec.cr @@ -168,45 +168,182 @@ describe XML do errors[0].to_s.should eq("Opening and ending tag mismatch: people line 1 and foo") end - it "gets root namespaces scopes" do - doc = XML.parse(<<-XML + describe "#namespace" do + describe "when the node has a namespace" do + describe "with a prefix" do + it "return the prefixed namespace" do + doc = XML.parse(<<-XML + + + XML + ) + + namespace = doc.root.not_nil!.namespace.should_not be_nil + namespace.href.should eq "http://a9.com/-/spec/opensearchrss/1.0/" + namespace.prefix.should eq "openSearch" + end + end + + describe "with a default prefix" do + it "return the default namespace" do + doc = XML.parse(<<-XML + + + XML + ) + + namespace = doc.root.not_nil!.namespace.should_not be_nil + namespace.href.should eq "http://a9.com/-/spec/opensearchrss/1.0/" + namespace.prefix.should be_nil + end + end + end + + describe "when the node does not have namespace" do + it "should return nil" do + doc = XML.parse(<<-XML + + + XML + ) + + doc.root.not_nil!.namespace.should be_nil + end + end + + describe "when the element does not have a namespace, but has namespace declarations" do + it "should return nil" do + doc = XML.parse(<<-XML + + + XML + ) + + doc.root.not_nil!.namespace.should be_nil + end + end + end + + describe "#namespace_definitions" do + it "returns namespaces explicitly defined" do + doc = XML.parse(<<-XML + XML - ) - namespaces = doc.root.not_nil!.namespace_scopes + ) - namespaces.size.should eq(2) - namespaces[0].href.should eq("http://www.w3.org/2005/Atom") - namespaces[0].prefix.should be_nil - namespaces[1].href.should eq("http://a9.com/-/spec/opensearchrss/1.0/") - namespaces[1].prefix.should eq("openSearch") + namespaces = doc.root.not_nil!.first_element_child.not_nil!.namespace_definitions + + namespaces.size.should eq(1) + namespaces[0].href.should eq("http://c") + namespaces[0].prefix.should eq "c" + end + + it "returns an empty array if no namespaces are defined" do + doc = XML.parse(<<-XML + + + + + XML + ) + + doc.root.not_nil!.first_element_child.not_nil!.namespace_definitions.should be_empty + end end - it "returns empty array if no namespaces scopes exists" do - doc = XML.parse(<<-XML + describe "#namespace_scopes" do + it "gets root namespaces scopes" do + doc = XML.parse(<<-XML + + + + XML + ) + namespaces = doc.root.not_nil!.namespace_scopes + + namespaces.size.should eq(2) + namespaces[0].href.should eq("http://www.w3.org/2005/Atom") + namespaces[0].prefix.should be_nil + namespaces[1].href.should eq("http://a9.com/-/spec/opensearchrss/1.0/") + namespaces[1].prefix.should eq("openSearch") + end + + it "returns empty array if no namespaces scopes exists" do + doc = XML.parse(<<-XML John XML - ) - namespaces = doc.root.not_nil!.namespace_scopes + ) + namespaces = doc.root.not_nil!.namespace_scopes - namespaces.size.should eq(0) + namespaces.size.should eq(0) + end + + it "includes parent namespaces" do + doc = XML.parse(<<-XML + + + + + XML + ) + namespaces = doc.root.not_nil!.first_element_child.not_nil!.namespace_scopes + + namespaces.size.should eq(3) + namespaces[0].href.should eq("http://c") + namespaces[0].prefix.should eq "c" + namespaces[1].href.should eq("http://www.w3.org/2005/Atom") + namespaces[1].prefix.should be_nil + namespaces[2].href.should eq("http://a9.com/-/spec/opensearchrss/1.0/") + namespaces[2].prefix.should eq("openSearch") + end end - it "gets root namespaces as hash" do - doc = XML.parse(<<-XML + describe "#namespaces" do + it "gets root namespaces as hash" do + doc = XML.parse(<<-XML XML - ) - namespaces = doc.root.not_nil!.namespaces - namespaces.should eq({ - "xmlns" => "http://www.w3.org/2005/Atom", - "xmlns:openSearch": "http://a9.com/-/spec/opensearchrss/1.0/", - }) + ) + namespaces = doc.root.not_nil!.namespaces + namespaces.should eq({ + "xmlns" => "http://www.w3.org/2005/Atom", + "xmlns:openSearch": "http://a9.com/-/spec/opensearchrss/1.0/", + }) + end + + it "includes parent namespaces" do + doc = XML.parse(<<-XML + + + + + XML + ) + namespaces = doc.root.not_nil!.first_element_child.not_nil!.namespaces + namespaces.should eq({ + "xmlns:c" => "http://c", + "xmlns" => "http://www.w3.org/2005/Atom", + "xmlns:openSearch": "http://a9.com/-/spec/opensearchrss/1.0/", + }) + end + + it "returns an empty hash if there are no namespaces" do + doc = XML.parse(<<-XML + + + + + XML + ) + namespaces = doc.root.not_nil!.first_element_child.not_nil!.namespaces + namespaces.should eq({} of String => String?) + end end it "reads big xml file (#1455)" do diff --git a/src/xml/node.cr b/src/xml/node.cr index cac2abac2bdb..8b0bd2670d5a 100644 --- a/src/xml/node.cr +++ b/src/xml/node.cr @@ -294,6 +294,20 @@ class XML::Node end end + # Returns namespaces defined on self element directly. + def namespace_definitions : Array(Namespace) + namespaces = [] of Namespace + + if ns = @node.value.ns_def + while ns + namespaces << Namespace.new(document, ns) + ns = ns.value.next + end + end + + namespaces + end + # Returns namespaces in scope for self – those defined on self element # directly or any ancestor node – as an `Array` of `XML::Namespace` objects. # @@ -304,13 +318,8 @@ class XML::Node def namespace_scopes : Array(Namespace) scopes = [] of Namespace - ns_list = LibXML.xmlGetNsList(@node.value.doc, @node) - - if ns_list - while ns_list.value - scopes << Namespace.new(document, ns_list.value) - ns_list += 1 - end + each_namespace do |namespace| + scopes << namespace end scopes From d68fc958cbbaa6197696c21b71fd28d03c0e9529 Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Sat, 7 Aug 2021 13:32:37 -0400 Subject: [PATCH 2/4] Add spec to assert the correct related namespace is returned --- spec/std/xml/xml_spec.cr | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/spec/std/xml/xml_spec.cr b/spec/std/xml/xml_spec.cr index fe5b410a18e6..b729f721b6cc 100644 --- a/spec/std/xml/xml_spec.cr +++ b/spec/std/xml/xml_spec.cr @@ -197,6 +197,29 @@ describe XML do namespace.prefix.should be_nil end end + + describe "without an explicit declaration on the node" do + it "returns the related namespace" do + doc = XML.parse(<<-XML + + + + + + XML + ) + + root = doc.root.not_nil! + + namespace = root.children[1].namespace.should_not be_nil + namespace.href.should eq "http://www.w3.org/2005/Atom" + namespace.prefix.should be_nil + + namespace = root.children[3].namespace.should_not be_nil + namespace.href.should eq "https://a-namespace" + namespace.prefix.should eq "a" + end + end end describe "when the node does not have namespace" do From ede5c006c01a8b834ce2041f9439d032d7bf8c77 Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Sat, 7 Aug 2021 14:56:34 -0400 Subject: [PATCH 3/4] PR suggestions --- spec/std/xml/xml_spec.cr | 114 +++++++++++++++++++-------------------- src/xml/node.cr | 9 ++-- 2 files changed, 61 insertions(+), 62 deletions(-) diff --git a/spec/std/xml/xml_spec.cr b/spec/std/xml/xml_spec.cr index b729f721b6cc..8ac267f64f7c 100644 --- a/spec/std/xml/xml_spec.cr +++ b/spec/std/xml/xml_spec.cr @@ -173,12 +173,12 @@ describe XML do describe "with a prefix" do it "return the prefixed namespace" do doc = XML.parse(<<-XML - - - XML + + + XML ) - namespace = doc.root.not_nil!.namespace.should_not be_nil + namespace = doc.root.not_nil!.namespace.should be_a XML::Namespace namespace.href.should eq "http://a9.com/-/spec/opensearchrss/1.0/" namespace.prefix.should eq "openSearch" end @@ -187,12 +187,12 @@ describe XML do describe "with a default prefix" do it "return the default namespace" do doc = XML.parse(<<-XML - - - XML + + + XML ) - namespace = doc.root.not_nil!.namespace.should_not be_nil + namespace = doc.root.not_nil!.namespace.should be_a XML::Namespace namespace.href.should eq "http://a9.com/-/spec/opensearchrss/1.0/" namespace.prefix.should be_nil end @@ -211,11 +211,11 @@ describe XML do root = doc.root.not_nil! - namespace = root.children[1].namespace.should_not be_nil + namespace = root.children[1].namespace.should be_a XML::Namespace namespace.href.should eq "http://www.w3.org/2005/Atom" namespace.prefix.should be_nil - namespace = root.children[3].namespace.should_not be_nil + namespace = root.children[3].namespace.should be_a XML::Namespace namespace.href.should eq "https://a-namespace" namespace.prefix.should eq "a" end @@ -225,9 +225,9 @@ describe XML do describe "when the node does not have namespace" do it "should return nil" do doc = XML.parse(<<-XML - - - XML + + + XML ) doc.root.not_nil!.namespace.should be_nil @@ -237,9 +237,9 @@ describe XML do describe "when the element does not have a namespace, but has namespace declarations" do it "should return nil" do doc = XML.parse(<<-XML - - - XML + + + XML ) doc.root.not_nil!.namespace.should be_nil @@ -250,11 +250,11 @@ describe XML do describe "#namespace_definitions" do it "returns namespaces explicitly defined" do doc = XML.parse(<<-XML - - - - - XML + + + + + XML ) namespaces = doc.root.not_nil!.first_element_child.not_nil!.namespace_definitions @@ -266,11 +266,11 @@ describe XML do it "returns an empty array if no namespaces are defined" do doc = XML.parse(<<-XML - - - - - XML + + + + + XML ) doc.root.not_nil!.first_element_child.not_nil!.namespace_definitions.should be_empty @@ -280,10 +280,10 @@ describe XML do describe "#namespace_scopes" do it "gets root namespaces scopes" do doc = XML.parse(<<-XML - - - - XML + + + + XML ) namespaces = doc.root.not_nil!.namespace_scopes @@ -296,9 +296,9 @@ describe XML do it "returns empty array if no namespaces scopes exists" do doc = XML.parse(<<-XML - - John - XML + + John + XML ) namespaces = doc.root.not_nil!.namespace_scopes @@ -307,11 +307,11 @@ describe XML do it "includes parent namespaces" do doc = XML.parse(<<-XML - - - - - XML + + + + + XML ) namespaces = doc.root.not_nil!.first_element_child.not_nil!.namespace_scopes @@ -328,41 +328,41 @@ describe XML do describe "#namespaces" do it "gets root namespaces as hash" do doc = XML.parse(<<-XML - - - - XML + + + + XML ) namespaces = doc.root.not_nil!.namespaces namespaces.should eq({ - "xmlns" => "http://www.w3.org/2005/Atom", - "xmlns:openSearch": "http://a9.com/-/spec/opensearchrss/1.0/", + "xmlns" => "http://www.w3.org/2005/Atom", + "xmlns:openSearch" => "http://a9.com/-/spec/opensearchrss/1.0/", }) end it "includes parent namespaces" do doc = XML.parse(<<-XML - - - - - XML + + + + + XML ) namespaces = doc.root.not_nil!.first_element_child.not_nil!.namespaces namespaces.should eq({ - "xmlns:c" => "http://c", - "xmlns" => "http://www.w3.org/2005/Atom", - "xmlns:openSearch": "http://a9.com/-/spec/opensearchrss/1.0/", + "xmlns:c" => "http://c", + "xmlns" => "http://www.w3.org/2005/Atom", + "xmlns:openSearch" => "http://a9.com/-/spec/opensearchrss/1.0/", }) end it "returns an empty hash if there are no namespaces" do doc = XML.parse(<<-XML - - - - - XML + + + + + XML ) namespaces = doc.root.not_nil!.first_element_child.not_nil!.namespaces namespaces.should eq({} of String => String?) diff --git a/src/xml/node.cr b/src/xml/node.cr index 8b0bd2670d5a..c3fa4d0dd26a 100644 --- a/src/xml/node.cr +++ b/src/xml/node.cr @@ -298,11 +298,10 @@ class XML::Node def namespace_definitions : Array(Namespace) namespaces = [] of Namespace - if ns = @node.value.ns_def - while ns - namespaces << Namespace.new(document, ns) - ns = ns.value.next - end + ns = @node.value.ns_def + while ns + namespaces << Namespace.new(document, ns) + ns = ns.value.next end namespaces From 590857a3b12356744b25466b4c54e10cda74adff Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Sat, 7 Aug 2021 15:17:07 -0400 Subject: [PATCH 4/4] Remove dangling `)` --- spec/std/xml/xml_spec.cr | 53 +++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 30 deletions(-) diff --git a/spec/std/xml/xml_spec.cr b/spec/std/xml/xml_spec.cr index 8ac267f64f7c..a2a6429db395 100644 --- a/spec/std/xml/xml_spec.cr +++ b/spec/std/xml/xml_spec.cr @@ -172,11 +172,10 @@ describe XML do describe "when the node has a namespace" do describe "with a prefix" do it "return the prefixed namespace" do - doc = XML.parse(<<-XML + doc = XML.parse(<<-XML) XML - ) namespace = doc.root.not_nil!.namespace.should be_a XML::Namespace namespace.href.should eq "http://a9.com/-/spec/opensearchrss/1.0/" @@ -186,11 +185,10 @@ describe XML do describe "with a default prefix" do it "return the default namespace" do - doc = XML.parse(<<-XML + doc = XML.parse(<<-XML) XML - ) namespace = doc.root.not_nil!.namespace.should be_a XML::Namespace namespace.href.should eq "http://a9.com/-/spec/opensearchrss/1.0/" @@ -200,14 +198,13 @@ describe XML do describe "without an explicit declaration on the node" do it "returns the related namespace" do - doc = XML.parse(<<-XML + doc = XML.parse(<<-XML) XML - ) root = doc.root.not_nil! @@ -224,11 +221,10 @@ describe XML do describe "when the node does not have namespace" do it "should return nil" do - doc = XML.parse(<<-XML + doc = XML.parse(<<-XML) XML - ) doc.root.not_nil!.namespace.should be_nil end @@ -236,11 +232,10 @@ describe XML do describe "when the element does not have a namespace, but has namespace declarations" do it "should return nil" do - doc = XML.parse(<<-XML + doc = XML.parse(<<-XML) XML - ) doc.root.not_nil!.namespace.should be_nil end @@ -249,13 +244,12 @@ describe XML do describe "#namespace_definitions" do it "returns namespaces explicitly defined" do - doc = XML.parse(<<-XML + doc = XML.parse(<<-XML) XML - ) namespaces = doc.root.not_nil!.first_element_child.not_nil!.namespace_definitions @@ -265,13 +259,12 @@ describe XML do end it "returns an empty array if no namespaces are defined" do - doc = XML.parse(<<-XML + doc = XML.parse(<<-XML) XML - ) doc.root.not_nil!.first_element_child.not_nil!.namespace_definitions.should be_empty end @@ -279,12 +272,12 @@ describe XML do describe "#namespace_scopes" do it "gets root namespaces scopes" do - doc = XML.parse(<<-XML + doc = XML.parse(<<-XML) XML - ) + namespaces = doc.root.not_nil!.namespace_scopes namespaces.size.should eq(2) @@ -295,24 +288,24 @@ describe XML do end it "returns empty array if no namespaces scopes exists" do - doc = XML.parse(<<-XML + doc = XML.parse(<<-XML) John XML - ) + namespaces = doc.root.not_nil!.namespace_scopes namespaces.size.should eq(0) end it "includes parent namespaces" do - doc = XML.parse(<<-XML + doc = XML.parse(<<-XML) XML - ) + namespaces = doc.root.not_nil!.first_element_child.not_nil!.namespace_scopes namespaces.size.should eq(3) @@ -327,12 +320,12 @@ describe XML do describe "#namespaces" do it "gets root namespaces as hash" do - doc = XML.parse(<<-XML + doc = XML.parse(<<-XML) XML - ) + namespaces = doc.root.not_nil!.namespaces namespaces.should eq({ "xmlns" => "http://www.w3.org/2005/Atom", @@ -341,13 +334,13 @@ describe XML do end it "includes parent namespaces" do - doc = XML.parse(<<-XML + doc = XML.parse(<<-XML) XML - ) + namespaces = doc.root.not_nil!.first_element_child.not_nil!.namespaces namespaces.should eq({ "xmlns:c" => "http://c", @@ -357,13 +350,13 @@ describe XML do end it "returns an empty hash if there are no namespaces" do - doc = XML.parse(<<-XML + doc = XML.parse(<<-XML) XML - ) + namespaces = doc.root.not_nil!.first_element_child.not_nil!.namespaces namespaces.should eq({} of String => String?) end @@ -377,11 +370,11 @@ describe XML do end it "sets node text/content" do - doc = XML.parse(<<-XML + doc = XML.parse(<<-XML) John XML - ) + root = doc.root.not_nil! root.text = "Peter" root.text.should eq("Peter") @@ -391,11 +384,11 @@ describe XML do end it "doesn't set invalid node content" do - doc = XML.parse(<<-XML + doc = XML.parse(<<-XML) John XML - ) + root = doc.root.not_nil! expect_raises(Exception, "Cannot escape") do root.content = "\0"