rewrite ResourceLinkParser

- use own RegExp instead of borrowing the ones from Grape routes
- add specs

lib/api/utilities/resource_link_parser.rb

@@ -30,27 +30,25 @@
 module API
   module Utilities
     module ResourceLinkParser
-      def self.parse(resource_link)
-        ::API::Root.routes.each do |route|
-          route_options = route.instance_variable_get(:@options)
+      # N.B. valid characters for URL path segments as of
+      # http://tools.ietf.org/html/rfc3986#section-3.3
+      SEGMENT_CHARACTER = '(\w|[-~!$&\'\(\)*+\.,:;=@]|%[0-9A-Fa-f]{2})'
+      RESOURCE_REGEX = "/api/v(?<version>\\d)/(?<namespace>\\w+)/(?<id>#{SEGMENT_CHARACTER}*)\\z"
 
-          # we are matching the resource link without trailing slashes, as they would still make
-          # for a valid link (even though it does not match the compiled regex)
-          match = route_options[:compiled].match(resource_link.chomp('/'))
+      def self.resource_matcher
+        @@matcher ||= Regexp.compile(RESOURCE_REGEX)
+      end
 
-          if match
-            # we want to capture the identifying key regardless of its name (e.g. :id)
-            id_key = match.names.reject { |name| ['version', 'format'].include?(name) }.first
-            id = id_key ? match[id_key] : nil
+      def self.parse(resource_link)
+        match = resource_matcher.match(resource_link)
 
-            return {
-              ns: /\/(?<ns>\w+)/.match(route_options[:namespace])[:ns],
-              id: id
-            }
-          end
-        end
+        return nil unless match
 
-        nil
+        return {
+          version: match[:version],
+          namespace: match[:namespace],
+          id: match[:id]
+        }
       end
     end
   end

lib/api/v3/render/render_api.rb

@@ -81,7 +81,7 @@ module API
               if params[:context]
                 context = parse_context
 
-                case context[:ns]
+                case context[:namespace]
                 when 'work_packages'
                   WorkPackage.visible(current_user).find(context[:id])
                 end
@@ -95,7 +95,8 @@ module API
                 fail API::Errors::InvalidRenderContext.new(
                   I18n.t('api_v3.errors.render.context_not_found')
                 )
-              elsif !SUPPORTED_CONTEXT_NAMESPACES.include? context[:ns]
+              elsif !SUPPORTED_CONTEXT_NAMESPACES.include?(context[:namespace]) ||
+                    context[:version] != '3'
                 fail API::Errors::InvalidRenderContext.new(
                   I18n.t('api_v3.errors.render.unsupported_context')
                 )

lib/api/v3/utilities/custom_field_injector.rb

@@ -114,8 +114,9 @@ module API
 
                             resource = ::API::Utilities::ResourceLinkParser.parse href
 
-                            if resource.nil? || resource[:ns] != expected_namespace
-                              actual_namespace = resource ? resource[:ns] : nil
+                            if resource.nil? || resource[:namespace] != expected_namespace ||
+                               resource[:version] != '3'
+                              actual_namespace = resource ? resource[:namespace] : nil
 
                               fail ::API::Errors::Form::InvalidResourceLink.new(property,
                                                                                 expected_namespace,

lib/api/v3/work_packages/form/work_package_attribute_links_representer.rb

@@ -127,8 +127,9 @@ module API
 
             resource = ::API::Utilities::ResourceLinkParser.parse href
 
-            if resource.nil? || resource[:ns] != ns.to_s
-              actual_ns = resource ? resource[:ns] : nil
+            if resource.nil? || resource[:namespace] != ns.to_s ||
+               resource[:version] != '3'
+              actual_ns = resource ? resource[:namespace] : nil
 
               property_localized = I18n.t("attributes.#{property}")
               expected_localized = I18n.t("attributes.#{ns.to_s.singularize}")

lib/api/v3/work_packages/link_to_object_extractor.rb

@@ -36,7 +36,7 @@ module API
             resource = ::API::Utilities::ResourceLinkParser.parse links[attribute]['href']
 
             if resource
-              case resource[:ns]
+              case resource[:namespace]
               when 'statuses'
                 h[:status_id] = resource[:id]
               end

spec/lib/api/utilities/resource_link_parser_spec.rb

@@ -0,0 +1,100 @@
+#-- copyright
+# OpenProject is a project management system.
+# Copyright (C) 2012-2015 the OpenProject Foundation (OPF)
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See doc/COPYRIGHT.rdoc for more details.
+#++
+
+require 'spec_helper'
+
+describe ::API::Utilities::ResourceLinkParser do
+  subject { described_class }
+
+  describe('#parse') do
+    shared_examples_for 'accepts resource link' do
+      it 'parses the version' do
+        expect(result[:version]).to eql(version)
+      end
+
+      it 'parses the namespace' do
+        expect(result[:namespace]).to eql(namespace)
+      end
+
+      it 'parses the id' do
+        expect(result[:id]).to eql(id)
+      end
+    end
+
+    shared_examples_for 'rejects resource link' do
+      it 'is nil' do
+        expect(result).to be_nil
+      end
+    end
+
+    describe 'accepts a simple resource' do
+      it_behaves_like 'accepts resource link' do
+        let(:result) { subject.parse '/api/v3/statuses/12' }
+        let(:version) { '3' }
+        let(:namespace) { 'statuses' }
+        let(:id) { '12' }
+      end
+    end
+
+    describe 'accepts all valid segment characters as id' do
+      it_behaves_like 'accepts resource link' do
+        let(:result) { subject.parse '/api/v3/string_object/foo-2_~!$&\'()*+.,:;=@%Fa' }
+        let(:version) { '3' }
+        let(:namespace) { 'string_object' }
+        let(:id) { 'foo-2_~!$&\'()*+.,:;=@%Fa' }
+      end
+    end
+
+    describe 'accepts resource with empty id segment' do
+      it_behaves_like 'accepts resource link' do
+        let(:result) { subject.parse '/api/v3/string_object/' }
+        let(:version) { '3' }
+        let(:namespace) { 'string_object' }
+        let(:id) { '' }
+      end
+    end
+
+    describe 'rejects resource with missing id segment' do
+      it_behaves_like 'rejects resource link' do
+        let(:result) { subject.parse '/api/v3/string_object' }
+      end
+    end
+
+    describe 'rejects the api root' do
+      it_behaves_like 'rejects resource link' do
+        let(:result) { subject.parse '/api/v3/' }
+      end
+    end
+
+    describe 'rejects nested resources' do
+      it_behaves_like 'rejects resource link' do
+        let(:result) { subject.parse '/api/v3/statuses/imaginary/' }
+      end
+    end
+  end
+end

spec/requests/api/v3/render_resource_spec.rb

@@ -102,11 +102,17 @@ describe 'API v3 Render resource' do
                               I18n.t('api_v3.errors.render.context_not_found')
             end
 
-            describe 'unsupported context found' do
+            describe 'unsupported context resource found' do
               let(:post_path) { "#{path}?context=/api/v3/activities/2" }
 
               it_behaves_like 'invalid render context', 'Unsupported context found.'
             end
+
+            describe 'unsupported context version found' do
+              let(:post_path) { "#{path}?context=/api/v4/work_packages/2" }
+
+              it_behaves_like 'invalid render context', 'Unsupported context found.'
+            end
           end
         end
       end