search

phaidra / phaidra-api

RESTish API to Phaidra
Apache License 2.0
6 stars 4 forks source link

getting members from non existing collection returns empty string instead of json #67

Closed yurj closed 1 year ago

yurj commented 2 years ago

https://github.com/phaidra/phaidra-api/blob/f8d8d59820784c40907f601ad6bfd04b7016c2a1/PhaidraAPI/Controller/Collection.pm#L295

for example:

$ curl -X GET "https://phaidratest.cab.unipd.it/api/collection/1/members"
$ 
$ curl -X GET "https://services.phaidra.univie.ac.at/api/collection/o:23213123/members"
$

the call return a json for non collection content types:

$ curl -X GET "https://services.phaidra.univie.ac.at/api/collection/o:1433423/members"
{"alerts":[],"metadata":{"members":[]}}$

the call should return at least an empty json ({}) but would be better to return:

{"alerts":[],"metadata":{"members":[]}}
giulioturetta commented 2 years ago

I guess return $r; is missing right after the line https://github.com/phaidra/phaidra-api/blob/master/PhaidraAPI/Model/Collection.pm#L138.

giulioturetta commented 2 years ago

Related log messages:

[2022/04/15 10:34:34] [ERROR] [6224] [error] Collection->get_members: Cannot get lastModifiedDate!
[2022/04/15 10:34:34] [ERROR] [6224] [error] must specify key at /usr/local/phaidra/phaidra-api/PhaidraAPI/Model/Collection.pm line 217.
giulioturetta commented 1 year ago

I also found that getting members of empty collections isn't cached, probably because $c->app->chi->set($cachekey, $cached_members, '1 day'); doesn't set the cache element when its value is undefined. I think it's not a big deal.