The version of SPYC that's bundled with Services 3.x is inconsistent between the 6.x (0.3) and 7.x (0.4.5) branches and is behind the latest 0.5.0 release. The attached patches update the REST server in 6.x and 7.x.

This also fixes a warning in 3.x-6.x that can be triggered by hitting endpoint/user/login.yaml with a GET, causing SPYC to dump an empty variable. 0.4.5 and 0.5.0 handle this fine but 0.3.0 produces a warning, hence the upgrade. There aren't any major interface changes between 0.4.5 and 0.5, so the 7.x patch just copies the upstream spyc.php into servers/rest_server/lib/spyc.php.

Files: 
CommentFileSizeAuthor
#12 spyc.php_.txt32.09 KBcotto
#7 services-spyc-object-array-fix.patch571 bytescotto
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in services-spyc-object-array-fix.patch.
[ View ]
rest_server_spyc_upgrade-6.x.patch35.79 KBcotto
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch rest_server_spyc_upgrade-6.x.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]
rest_server_spyc_upgrade-7.x.patch6.12 KBcotto
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch rest_server_spyc_upgrade-7.x.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

marcingy’s picture

Category:bug» task
marcingy’s picture

Status:Active» Postponed

We are going to remove these libraries #1325572: Integrate services with the libraries module to improve handling of third party libraries.. They should never have been in repo in the first place.

marcingy’s picture

Status:Postponed» Closed (won't fix)

Libraries no longer ship with services

kylebrowning’s picture

Status:Closed (won't fix)» Active

Yaml is currently broken, this needs to happen.

marcingy’s picture

We need to update d6 to work with the latest version of spyc rather than committing the entire patch.

marcingy’s picture

This this should be in head - http://drupal.org/node/1334784

cotto’s picture

StatusFileSize
new571 bytes
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in services-spyc-object-array-fix.patch.
[ View ]

marcingy, that doesn't seem to be the problem. It looks like the root problem spyc doesn't know how to serialize an array of objects. The following code will also fail.

    $spyc = new Spyc;
    $array = array( );
    $array[0] = new StdClass;
    $array[0]->foo = 'buz';
    return $spyc->dump($array, 0, 0);

The fix for spyc is pretty simple (attached). I've also submitted an upstream issue with the same patch. It's likely to be a while before someone upstream gets around to fixing the issue, so we should probably figure out what to do in the meantime.

marcingy’s picture

I am not sure how to handle this let me think some more! I'm going to show some ignorance around this library can we make sure we supply an array or are there cases where we need to provide an object. I ask as all data being passed to services should be array anyway.

cotto’s picture

The issue isn't the data that's being passed to services, but the data that's being returned.
Converting anything that'll get serialized through spyc into an array is possible, though clunky. I don't see any reason why objects strictly need to be part of the output, but it also seems weak reformat services' data because of a simple upstream bug in spyc.

The best way to deal with this may be to include a patched spyc with Services. If a similar patch gets integrated into upstream spyc, services can go back to requiring the download as it does now.

apotek’s picture

The best way to deal with this may be to include a patched spyc with Services. If a similar patch gets integrated into upstream spyc, services can go back to requiring the download as it does now.

I think if someone can post a patched version of Spyc in this thread, that is all that needs to be done. Services should not be including the spyc library in its download. Maybe a posted patch, and a link from the main project description page to that patched version of spyc would tide users over until spyc fixes itself upstream.

marcingy’s picture

Version:6.x-3.x-dev» 7.x-3.x-dev

The approach described by @apotek sounds like the best option. Moving to d7 as we need fix there as well I'm guessing.

cotto’s picture

StatusFileSize
new32.09 KB

Here's the patched version of spyc, renamed due to d.o's filename extension limits.

apotek’s picture

Thanks! This looks good to me and I've committed this into production.

marcingy’s picture

Status:Active» Reviewed & tested by the community

Marking as rtbc note do not commit this patch ;) Just noting it has sign off and works. I have added a link to the on the main page to this issue.

rypit’s picture

Does it make sense to use the Libraries API for all external libraries? library_get_path seems like a nice way to organize things moving forward. It seems like it's a very organized way to handle libraries for multisites especially. Should this be a feature request?

marcingy’s picture

#15 is total different issue #1325572: Integrate services with the libraries module to improve handling of third party libraries. and nothing to do with the problem here and already has an item in the services queue and patch that needs work. Whether libraries is used or not there is still an upstream issue sypc and this library should not exist any on d.o in a committed state.

kylebrowning’s picture

Status:Reviewed & tested by the community» Closed (won't fix)

Im marking this as won't fix, id like #15 to push forward asap.

pcoughlin’s picture

Status:Closed (won't fix)» Needs review

I am still not able to get the Status Report warning to go away. Have put spyc.php in every folder I can find a reference to and the warning is still present.

marcingy’s picture

wal-78’s picture

rest_server_spyc_upgrade-7.x.patch queued for re-testing.

marcingy’s picture

Status:Needs review» Closed (won't fix)

Please do not re-open old issues this was solved in a different way using libraries api.