Rubocop XML module files and update method references

lib/calligraphy.rb

@@ -5,6 +5,7 @@ require 'calligraphy/rails/web_dav_methods'
 require 'calligraphy/rails/web_dav_preconditions'
 require 'calligraphy/rails/web_dav_requests_controller'
 
+require 'calligraphy/xml/web_dav_elements'
 require 'calligraphy/xml/builder'
 require 'calligraphy/xml/namespace'
 require 'calligraphy/xml/node'

lib/calligraphy/web_dav_request/lock.rb

@@ -8,6 +8,7 @@ module Calligraphy
 
     attr_reader :resource_exists
 
+    #:nodoc:
     def initialize(headers:, request:, response:, resource:)
       super
 
@@ -59,7 +60,7 @@ module Calligraphy
 
     def build_response(lock_properties)
       builder = xml_builder
-      xml_res = builder.lock_res lock_properties
+      xml_res = builder.lock_response lock_properties
 
       lock_token = extract_lock_token lock_properties
       prepare_response_headers lock_token

lib/calligraphy/web_dav_request/propfind.rb

@@ -13,7 +13,8 @@ module Calligraphy
       properties = @resource.propfind xml
 
       builder = xml_builder
-      xml_res = builder.propfind_res @resource.full_request_path, properties
+      xml_res = builder.propfind_response(@resource.full_request_path,
+                                          properties)
 
       set_xml_content_type
 

lib/calligraphy/web_dav_request/proppatch.rb

@@ -18,7 +18,8 @@ module Calligraphy
       actions = @resource.proppatch xml
 
       builder = xml_builder
-      xml_res = builder.proppatch_res @resource.full_request_path, actions
+      xml_res = builder.proppatch_response(@resource.full_request_path,
+                                           actions)
 
       set_xml_content_type
 

lib/calligraphy/xml/builder.rb

@@ -1,167 +1,118 @@
-module Calligraphy::XML
+# frozen_string_literal: true
+
+module Calligraphy
+  module XML
+    # Responsible for building XML responses for WebDAV requests.
     class Builder
-    SUPPORTED_NS_TAGS = %w(
+      include Calligraphy::XML::WebDavElements
-      creationdate displayname exclusive getcontentlanguage getcontentlength
-      getcontenttype getetag getlastmodified href lockdiscovery lockscope
-      locktype owner write
-    )
 
       attr_reader :dav_ns, :default_ns, :server_protocol
 
+      #:nodoc:
       def initialize(dav_ns: 'D', server_protocol: 'HTTP/1.1')
         @dav_ns = dav_ns
         @default_ns = { "xmlns:#{@dav_ns}" => 'DAV:' }
         @server_protocol = server_protocol
       end
 
-    def lock_res(activelock_properties)
-      build :prop do |xml|
-        xml.lockdiscovery do
-          activelock_properties.each do |properties|
-            activelock xml, properties
-          end
-        end
-      end
-    end
-
-    def propfind_res(path, properties)
-      multistatus do |xml|
-        href xml, path
-        propstat xml, properties[:found], :ok
-        propstat xml, properties[:not_found], :not_found
-      end
-    end
-
-    def proppatch_res(path, actions)
-      multistatus do |xml|
-        href xml, path
-        propstat xml, actions[:set]
-        propstat xml, actions[:remove]
-      end
-    end
-
       private
 
       def build(tag)
         Nokogiri::XML::Builder.new do |xml|
-        xml[@dav_ns].send(tag, @default_ns) do
+          xml[@dav_ns].send(tag, @default_ns) { yield xml }
-          yield xml
-        end
         end.to_xml
       end
 
-    def activelock(xml, property_set)
-      xml.activelock do
-        property_set.each do |property|
-          property_drilldown xml, property
-        end
-      end
-    end
-
-    def href(xml, path)
-      xml.href path
-    end
-
       def multistatus
         build :multistatus do |xml|
-        xml.response do
+          xml.response { yield xml }
-          yield xml
-        end
-      end
-    end
-
-    def prop(xml, property_set)
-      xml.prop do
-        property_set.each do |property|
-          property_drilldown xml, property
-        end
-      end
-    end
-
-    def propstat(xml, property_set, status=:ok)
-      return unless property_set.length > 0
-
-      xml.propstat do
-        prop xml, property_set
-        status xml, status
-      end
-    end
-
-    def resourcetype(xml, property)
-      if property.children.text == 'collection'
-        xml[@dav_ns].resourcetype do
-          xml.send 'collection'
-        end
-      else
-        xml[@dav_ns].resourcetype
-      end
-    end
-
-    def status(xml, status)
-      xml.status status_message status
-    end
-
-    def supportedlock(xml, property)
-      children = JSON.parse property.text, symbolize_names: true
-
-      xml[@dav_ns].supportedlock do
-        children.each do |child|
-          xml[@dav_ns].lockentry do
-            xml[@dav_ns].lockscope do
-              xml.text child[:lockentry][:lockscope]
-            end
-
-            xml[@dav_ns].locktype do
-              xml.text child[:lockentry][:locktype]
-            end
-          end
-        end
-      end
-    end
-
-    # NOTE: `xml[@dav_ns].send timeout` results in Timeout being called, so
-    # we have this timeout method for convenience
-    def timeout(xml, property)
-      xml[@dav_ns].timeout do
-        xml.text property.text
         end
       end
 
       def property_drilldown(xml, property)
         if property.is_a? Array
-        property.each do |prop|
+          iterate_and_drilldown xml, property
-          property_drilldown xml, prop
+        elsif DAV_NS_TAGS.include? property.name
-        end
+          supported_ns_tag xml, property
-      elsif property.children && property.text.nil?
+        elsif property.namespace&.href
-        xml.send property.name do
+          non_supported_ns_tag xml, property
-          property.children.each do |child|
-            property_drilldown xml, child
-          end
-        end
-      elsif property.name == 'resourcetype'
-        resourcetype xml, property
-      elsif property.name == 'supportedlock'
-        supportedlock xml, property
-      elsif property.name == 'timeout'
-        timeout xml, property
-      elsif SUPPORTED_NS_TAGS.include? property.name
-        xml[@dav_ns].send property.name do
-          xml.text property.text
-        end
-      elsif property.namespace && property.namespace.href
-        xml.send property.name, xmlns: property.namespace.href do
-          xml.text property.text
-        end
         else
-        xml.send property.name, property.text do
+          nil_ns_tag xml, property
-          xml.parent.namespace = nil
+        end
+      end
+
+      def iterate_and_drilldown(xml, property_set)
+        property_set.each do |property|
+          property_drilldown xml, property
+        end
+      end
+
+      def supported_ns_tag(xml, property)
+        if DAV_NS_METHODS.include? property.name
+          return send property.name, xml, property
+        end
+
+        xml[@dav_ns].send property.name do
+          if property.children
+            iterate_and_drilldown xml, property.children
+          else
+            xml.text property.text
           end
         end
       end
 
+      def non_supported_ns_tag(xml, property)
+        xml.send property.name, xmlns: property.namespace.href do
+          if property.children
+            iterate_and_drilldown xml, property.children
+          else
+            xml.text property.text
+          end
+        end
+      end
+
+      def nil_ns_tag(xml, property)
+        xml.send property.name, property.text do
+          xml.parent.namespace = nil
+        end
+      end
+
+      def self_closing_tag(xml, text)
+        xml.send text
+      end
+
+      def href(xml, path)
+        xml[@dav_ns].href path
+      end
+
+      def prop(xml, property_set)
+        xml[@dav_ns].prop do
+          iterate_and_drilldown xml, property_set
+        end
+      end
+
+      def propstat(xml, property_set, status = :ok)
+        return if property_set.empty?
+
+        xml[@dav_ns].propstat do
+          prop xml, property_set
+          status xml, status
+        end
+      end
+
+      def status(xml, status)
+        xml[@dav_ns].status status_message status
+      end
+
       def status_message(status)
         status_code = Rack::Utils.status_code status
-      [@server_protocol, status_code, Rack::Utils::HTTP_STATUS_CODES[status_code]].join ' '
+
+        [
+          @server_protocol,
+          status_code,
+          Rack::Utils::HTTP_STATUS_CODES[status_code]
+        ].join ' '
+      end
     end
   end
 end

lib/calligraphy/xml/namespace.rb

@@ -1,10 +1,16 @@
-module Calligraphy::XML
+# frozen_string_literal: true
+
+module Calligraphy
+  module XML
+    # Simple XML namespace, used to store a namespace's href and prefix values.
     class Namespace
       attr_accessor :href, :prefix
 
+      #:nodoc:
       def initialize(namespace)
         @href = namespace.href if namespace.href
         @prefix = namespace.prefix if namespace.prefix
       end
     end
   end
+end

lib/calligraphy/xml/node.rb

@@ -1,9 +1,16 @@
-module Calligraphy::XML
+# frozen_string_literal: true
+
+module Calligraphy
+  module XML
+    # Simple XML node, used to store resource properties in Resource methods
+    # and later to create XML response bodies.
     class Node
       attr_accessor :children, :name, :namespace, :text
 
+      #:nodoc:
       def initialize(node = nil)
-      unless node.nil?
+        return if node.nil?
+
         @name = node.name
         @text = node.text unless node.text.empty?
 
@@ -11,12 +18,18 @@ module Calligraphy::XML
           @namespace = Calligraphy::XML::Namespace.new node.namespace
         end
 
-        if node.children&.length > 0
+        return unless node_has_children node
+
         @children = []
-          node.children.each do |child|
+        node.children.each { |x| @children.push Calligraphy::XML::Node.new x }
-            @children.push Calligraphy::XML::Node.new child
+      end
-          end
+
-        end
+      private
+
+      def node_has_children(node)
+        return false if node.children.nil?
+
+        node.children.length.positive?
       end
     end
   end

lib/calligraphy/xml/utils.rb

@@ -1,18 +1,29 @@
-module Calligraphy::XML
+# frozen_string_literal: true
+
+module Calligraphy
+  module XML
+    # Miscellaneous XML convenience methods.
     module Utils
+      # Returns the XML for a given XML body and node/CSS selector.
       def xml_for(body:, node:)
         xml = Nokogiri::XML body
         return :bad_request unless xml.errors.empty?
 
         namespace = nil
         xml.root.namespace_definitions.each do |n|
-        namespace = "#{n.prefix}|" if n&.href == Calligraphy::DAV_NS && !n.prefix.nil?
+          namespace = "#{n.prefix}|" if dav_namespace n
         end
-      namespace
 
         node = node.split(' ').map! { |n| namespace + n }.join(' ') if namespace
 
         xml.css(node).children
       end
+
+      private
+
+      def dav_namespace(namespace)
+        namespace&.href == Calligraphy::DAV_NS && !namespace.prefix.nil?
+      end
+    end
   end
 end

lib/calligraphy/xml/web_dav_elements.rb

@@ -0,0 +1,92 @@
+# frozen_string_literal: true
+
+module Calligraphy
+  module XML
+    # Methods to help build WebDAV elements and properties.
+    module WebDavElements
+      DAV_NS_TAGS = %w[
+        activelock allprop collection creationdate depth displayname error
+        exclusive getcontentlanguage getcontentlength getcontenttype getetag
+        getlastmodified href include location lockdiscovery lockentry lockinfo
+        lockroot lockscope locktoken locktype multistatus owner prop
+        propertyupdate propfind propname propstat remove response
+        responsedescription resourcetype set shared status supportedlock
+        timeout write
+      ].freeze
+
+      DAV_NS_METHODS = %w[resourcetype supportedlock timeout].freeze
+
+      # Build an XML response for a LOCK request.
+      def lock_response(activelock_properties)
+        build :prop do |xml|
+          xml.lockdiscovery do
+            activelock_properties.each do |properties|
+              xml.activelock do
+                iterate_and_drilldown xml, properties
+              end
+            end
+          end
+        end
+      end
+
+      # Build an XML response for a PROPFIND request.
+      def propfind_response(path, properties)
+        multistatus do |xml|
+          href xml, path
+          propstat xml, properties[:found], :ok
+          propstat xml, properties[:not_found], :not_found
+        end
+      end
+
+      # Build an XML response for a PROPPATCH request.
+      def proppatch_response(path, actions)
+        multistatus do |xml|
+          href xml, path
+          propstat xml, actions[:set]
+          propstat xml, actions[:remove]
+        end
+      end
+
+      private
+
+      def resourcetype(xml, property)
+        xml[@dav_ns].resourcetype do
+          self_closing_tag xml, property.text if property.text == 'collection'
+        end
+      end
+
+      def supportedlock(xml, property)
+        children = JSON.parse property.text, symbolize_names: true
+
+        xml[@dav_ns].supportedlock do
+          children.each do |child|
+            xml[@dav_ns].lockentry do
+              lockscope xml, child[:lockentry][:lockscope]
+              locktype xml, child[:lockentry][:locktype]
+            end
+          end
+        end
+      end
+
+      def lockscope(xml, scope)
+        xml[@dav_ns].lockscope do
+          self_closing_tag xml, scope
+        end
+      end
+
+      def locktype(xml, type)
+        xml[@dav_ns].locktype do
+          self_closing_tag xml, type
+        end
+      end
+
+      # NOTE: `xml[@dav_ns].send timeout` results in Timeout being called, so
+      # we have this timeout method for convenience.
+      def timeout(xml, property)
+        xml[@dav_ns].timeout do
+          xml.text property.text
+        end
+      end
+    end
+  end
+end