We need to add validation if the path already exists in already defined resources.

CommentFileSizeAuthor
#4 services_views-1688272-validate-resource-path-5.patch1.76 KBasgorobets
#2 services_views-1688272-validate-resource-path-2.patch1.94 KBasgorobets

Comments

asgorobets’s picture

asgorobets commented
Assigned: Unassigned » asgorobets
asgorobets’s picture

asgorobets commented
StatusFileSize
new services_views-1688272-validate-resource-path-2.patch1.94 KB
  • Check path for a valid resource name. Should contain alphanumeric characters, '-' or '_'.
  • Check if a path is already in use.
ygerasimov’s picture

ygerasimov commented
Status: Active » Needs work 
+++ b/includes/views/views_plugin_display_services.incundefined
@@ -11,6 +11,8 @@
+  function has_path() { return TRUE; }

why do we need to implement has_path()?

+++ b/includes/views/views_plugin_display_services.incundefined
@@ -111,6 +113,32 @@ class views_plugin_display_services extends views_plugin_display {
+  function options_validate(&$form, &$form_state) {
+    // It is very important to call the parent function here:
+    parent::options_validate($form, $form_state);

Comment should end with dot.

+++ b/includes/views/views_plugin_display_services.incundefined
@@ -111,6 +113,32 @@ class views_plugin_display_services extends views_plugin_display {
+  function options_validate(&$form, &$form_state) {
+    // It is very important to call the parent function here:
+    parent::options_validate($form, $form_state);
+    switch ($form_state['section']) {
+      case 'path':
+        ¶
+        if (preg_match('/[^a-zA-Z0-9-_]+/', $form_state['values']['path'])) {
+          form_error($form['path'], t('Use only alphanumeric characters, "-" and "_" for resource name'));
+        }
+        ¶
+        // Get current path and see if it was changed
+        $current_path = $this->get_option('path');
+        if ($current_path != $form_state['values']['path']) {
+          // We build resources from database to bypass cache
+          module_load_include('resource_build.inc', 'services');
+          $resources = _services_build_resources();
+          ¶
+          if (array_key_exists($form_state['values']['path'], $resources)) {
+            form_error($form['path'], t('Resource with this path already exists.'));
+          }  ¶
+        } ¶
+        ¶
+        break;

No need for switch construction. Simple "if" will do the job here.

+  
+  function validate() {
+    $errors = parent::validate();
+   
+    return $errors;
+  }

Why do we need this method?

asgorobets’s picture

asgorobets commented
StatusFileSize
new services_views-1688272-validate-resource-path-5.patch1.76 KB

function has_path() { return TRUE; }
- this will say display parent validate function to validate empty paths. This is needed in case we create a new display, default path will be '/', we need to avoid empty paths.

switch statement was changed to if

we don't need to redeclare validate(), has been removed

added dots to comments

ygerasimov’s picture

ygerasimov commented
Status: Needs work » Fixed

Committed. Thank you! c4dfac1

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

alex.skrypnyk’s picture

alex.skrypnyk
he/him
Primary language English
Location Melbourne
commented

The regexp in provided patch does not allow to add dot (.) as the part of the path (canonical name). This leads to the problem with a format of returned data in case if requester did not specify header 'Accept' and value.

To determine in which format to return results, Services uses either the extension of canonical name OR value of 'Accept' header. If none of them are set - the behaviour is not predictable.

Example of such problem would be a service path /services/rest/some-feed not returning correct results for IE6-9 and any other custom parsers that do not set header.

The solution is to add '.' into regular expression provided in the patch, so the line 

if (preg_match('/[^a-zA-Z0-9-_]+/', $form_state['values']['path'])) {

becomes

if (preg_match('/[^a-zA-Z0-9-_.]+/', $form_state['values']['path'])) {