/**
 * Return the location of the spyc library, if any.
 *
 * @return the location of the spyc library, if it exists; else return false.
 */
function _rest_server_get_spyc_location() {
  if (function_exists('libraries_get_path')) {
    $libraries_spyc_loc = libraries_get_path('spyc') . '/spyc.php'; 
  } else {
    return false;
  }
  $services_spyc_loc = dirname(__FILE__) . '/lib/spyc.php';

  if (file_exists($libraries_spyc_loc)) {
    return $libraries_spyc_loc;
  }
  else if (file_exists($services_spyc_loc)) {
    return $services_spyc_loc;
  }
  return false;
}

If libraries_get_path doesn't exist this always returns false.

2 solutions:
---------------

  1. Remove the else with the return false
  2. Remove all the other code and make a dependency on the libraries module
CommentFileSizeAuthor
#8 1476788-spyc-detect-refactor.patch1.21 KBcotto
#4 1476788-logic-error-4.patch987 bytesaspilicious
#3 1476788-logic-error-3.patch987 bytesaspilicious

Comments

cotto’s picture

cotto commented

That does look like a logic error. Thanks for reporting. If you have the tuits, a patch would be appreciated. Otherwise, it should be a pretty straightforward fix.

aspilicious’s picture

aspilicious commented
Assigned: Unassigned » aspilicious

I'm going to patch this today. Just removing the else to prevent a dependency on libraries.

aspilicious’s picture

aspilicious commented
Status: Active » Needs review
StatusFileSize
new 1476788-logic-error-3.patch987 bytes
aspilicious’s picture

aspilicious commented
StatusFileSize
new 1476788-logic-error-4.patch987 bytes

This one is better I think, we always have to ensure there is something in $libraries_spyc_loc, else we possible get notices when libraries is not installed.

marcingy’s picture

marcingy commented

The above patch looks good but to be honest that entire function could be refactored and cleaned up on a second look at the code I am happy for #4 to be committed but will try and role a follow up when I am at my desktop but basically

function _rest_server_get_spyc_location() { 
  if (function_exists('libraries_get_path')) {    
    $libraries_spyc_loc = libraries_get_path('spyc') . '/spyc.php';
    if (file_exists($libraries_spyc_loc)) {    
       return $libraries_spyc_loc;  
    }   
  }
  
  // Libraries not in use fall back to straight look up.
  $services_spyc_loc = dirname(__FILE__) . '/lib/spyc.php';  
  return file_exists($services_spyc_loc) ? $services_spyc_loc : false;
}
marcingy’s picture

marcingy commented
Status: Needs review » Needs work

Actually patch in #4 will still give notices as it stands after further review of the code.

aspilicious’s picture

aspilicious commented

Your code looks good, push that and I'm happy :)

cotto’s picture

cotto commented
StatusFileSize
new 1476788-spyc-detect-refactor.patch1.21 KB

Here's a patch with marcingy's changes against 7.x-3.x. I've tested both with Libraries installed and with the conditional hard-coded to check for a non-existent function, both with and without the spyc library installed. All tests pass in all cases.

cotto’s picture

cotto commented
Status: Needs work » Needs review
aspilicious’s picture

aspilicious commented
Status: Needs review » Needs work 
+++ b/servers/rest_server/rest_server.moduleundefined
@@ -219,18 +219,14 @@ function rest_server_services_resources_alter(&$resources, $endpoint) {
+  if (function_exists('libraries_get_path')) {   ¶

Trailing whitespaces

On every line

marcingy’s picture

marcingy commented
Version: 7.x-3.x-dev » 6.x-3.x-dev
Status: Needs work » Patch (to be ported)

White space fixed and committed to d7, I believe this needs to be ported to d6.

Thanks to everyones help on this.

cotto’s picture

cotto commented
Version: 6.x-3.x-dev » 7.x-3.x-dev
Status: Patch (to be ported) » Fixed

The part of the Libraries api we use is the same between 6.x and 7.x, so I've cherry-picked the commit into 6.x-3.x. Unless something horribly breaks, we can call this closed. aspilicious, thanks for reporting.

Status: Fixed » Closed (fixed)

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