Collection Service Implementation #140
Conversation
Implements Collection Service with RDF aggregator. Clean up of composer.json to use 0.0.2 chullo package Allows Resource Service Config yaml files to be overridden by implementing app Adds a common route collection to ResourceService unifying some middleware
In case Content-type is empty default to text/turtle. Important even when nothing is passed in the body
|
Awesome @DiegoPino, thanks for throwing this up. |
|
@daniel-dgi thanks. It's kinda monolith, all the code inside the route itself. But on the other side i had to do a lot of header smashing and massaging so for the time being i'm happy with this. Even some static controller invocation in place (noticed that silex is faster than triple indexing!) @Islandora-CLAW/7-x-2-x-committers i hope you enjoy some extra headers in the response 😄 |
|
@DiegoPino That's not the end of the world. We can refactor that into a service with an interface without too much hassle. Def want to keep those controllers thin so we have DI flexibility. I can help you out with that if you're agreeable to a pull on pull. |
|
@daniel-dgi, i think we can have this as part of the next sprint also. Since object service will be very similar and we could reuse some logic. Could be a good work for new people. I'm also ok with pulls agains pulls. I love pulls. |
| ->value('id',""); | ||
|
|
||
| $app->after(function (Request $request, Response $response) use ($app) { | ||
| $response->headers->set('X-Powered-By', 'Islandora Collection REST API v'.$app['config']['islandora']['apiVersion'], TRUE); //Nice |
|
@DiegoPino I'll pull it down and give it a whirl as soon as possible. There's tiny things I could nitpick, like str_replace instead of preg_replace, etc.... But that's kinda pointless. If the code works and is good, I'm happy merging and we can make other tickets to refactor. Haven't checked, but a good phpcbf might be in order too. |
|
@daniel-dgi, i'm still confused about silex coding standards. There are recommendations i try to follow(and i my forget a lot) but the examples from symphony are very "ample" in this sense. Also most of the providers i have seen don't use interfaces, so i skipped them also. Oh, yea, str_replace is fine. Just pull against whatever you find weird. thanks! |
|
@DiegoPino I just use PSR-2 for everything PHP outside of Drupal. The idea of the interface is just future proofing for later without breaking everything. More of a core tenant of DI than anything else. If we've made it, and we're injecting it, it probably needs an interface. Then we can make services that have alternate implementations (like querying something other than a 3-store) without wrecking everything or having to touch up the controller. Again, bit much for now. We'll just make seperate tickets. If you want, i'll even assign it to myself just to show folks what I mean. And I'm not gonna harp on you about str_replace vs. preg_replace. For reals. It's that kind of annoying stuff that discourages contribution. At the end of the day, if a regex turns out to be the performance bottleneck, I'll eat my hat. Or should I say my chullo? :D |
| @@ -1,8 +1,11 @@ | |||
| islandora: | |||
| apiVersion: 0.0.1 | |||
| fedoraProtocol: http | |||
There was a problem hiding this comment.
What is the apiVersion number tied to?
There was a problem hiding this comment.
@ruebot, just an idea i had. Since our api is tied to branch name right now maybe that numbering makes no sense. Any suggestions? thanks!
There was a problem hiding this comment.
No suggestions now. Just curious what it was.
Thanks to good @ruebot suggestion.
According to @daniel-dgi suggestion. I hope i did not break anything!
|
Tested works as expected 👍 . We can merge this and push on to adding additional routes.
Edit: My mistake, you can skip the UUID to have a collection at the root of the Respository and the request accepts an RDF document which can have a dc:title or more in it. |
|
|
Profit? As in a financial gain? |
|
The bullet list with ??? reminded me of the south park underpants gnomes meme 😄 |
|
I had to look it up, but now I think our project plan should involve the step.
|
|
|
||
| $urlRoute = $request->getUriForPath('/islandora/resource/'); | ||
|
|
||
| $subRequestPost = Request::create($urlRoute.$id, 'POST', array(), $request->cookies->all(), array(), $request->server->all(), $pcmd_collection_rdf); |
There was a problem hiding this comment.
Why subrequest when you can just use chullo?
There was a problem hiding this comment.
Lots of reasons, we get all the middleware ,like checking if the base resource is there, header rewriting and we don't need to rewrite psr7 responses. More over, the idea is to not use chullo directly. ResourceService should deal with any new chullo functionality if needed (if we for example need to recode) but not in this higher level services. And since we already using all the other services ResourceService provides, going to chullo when we already have the routes is sub-using our code.
|
@whikloj, the service already creates a collection at root if /$uuid part of the path is not passed |
|
As soon as I wrote that I realized that you were passing the Request object which would contain the body. My mistake, going to try that out too. |
|
@whikloj, my mistake in the pull description. I will paste an rdf example in the pull description so other people know how to test with |
|
Tested with putting a Collection at root with a small RDF document. Worked as expected. 👍 |
|
@whikloj, thanks por testing. Could you strikethrough the missing functionality so other people don't get confused when reading this? Muchas gracias! |
|
@whikloj, @daniel-dgi, @ruebot. If you look at the response headers there is a 'link' one too, pointing to a valid islandora claw resource service path.I like the idea we advertise this way how to access the created resource (and to facilitate adding members to the collection once created). I could also append in the future the tx there (i would love the tx would be in some header…is there one suitable for this?) |
|
Here is my request and response for posterity. |
|
Actually just thinking, should this return a |
|
@whikloj, yeah, should be 201, maybe i did something wrong there. I will check (and pull), because i'm returning what i get from the indirect container PUT? mmm… good catch! Update: my fault. fixed!! |
@whikloj thanks for catching this
|
For the TX stuff. maybe adding it to Session-Id: header? |
|
Keep getting this when trying to composer install, despite having composer install'd the ResourceServiceProvider and providing it's full path in composer.json: Problem 1 Seen this one before, guys? |
|
Yeah I had that, in composer.json change the require from: or whatever you named @DiegoPino's branch |
|
Yup, there we go. Knew I was derpin' on that somehwere |
|
404's when not providing a message body. Is that expected behaviour? Figured we'd just make a collection with no title and slap a uuid on there. |
|
@daniel-dgi, are you sure? I get curl -v -XPOST "http://localhost:8181/islandora/collection"
* Hostname was NOT found in DNS cachellection"
* Trying ::1...
* Connected to localhost (::1) port 8181 (#0)
> POST /islandora/collection HTTP/1.1
> User-Agent: curl/7.38.0
> Host: localhost:8181
> Accept: */*
>
< HTTP/1.1 201 Created
< Host: localhost:8181
< Connection: close
< X-Powered-By: PHP/5.6.16
< Date: Fri, 12 Feb 2016 20:09:01 GMT
< ETag: "896ccfa1c476bcf06967e1fed3e1d55eba176770"
< Cache-Control: private, must-revalidate
< Last-Modified: Fri, 12 Feb 2016 20:09:01 GMT
< Location: http://localhost:8080/rest/d2/c5/03/13/d2c50313-c407-442d-9add-d3ed01bbd9fa/members
< Content-Type: text/plain; charset=UTF-8
< Content-Length: 83
* Server Jetty(9.2.3.v20140905) is not blacklisted
< Server: Jetty(9.2.3.v20140905)
< Link: <http://localhost:8181/islandora/resource/ed05fae2-bcb4-461f-b2d0-251a177da1db/members>; rel="alternate"
< X-Powered-By: Islandora Collection REST API v0.0.1
<
* Closing connection 0
http://localhost:8080/rest/d2/c5/03/13/d2c50313-c407-442d-9add-d3ed01bbd9fa/members |
|
Getting this ~ $ curl -v -X POST "http://127.0.0.1:12345/islandora/collection"
Then in the actual message output, i'm getting Silex\Application">Application->abort('404', 'Failed creating PCDM Collection') |
|
@daniel-dgi. It's not a bug:
Solution check that your settings.dev.yml and/or settings.yml match the fedora endpoint you are using. |
|
You were close @DiegoPino. I thought i had set those values correctly, but was having weird issues with not being able to resolve localhost. 127.0.0.1 worked just fine. That's definitely just an env issue on my end. I've got this working, and it's doing what you say it does. |
Collection Service Implementation
|
@daniel-dgi cool! Yeah i had this localhost issues somewhere in the past. Thanks for testing and merging 👍 |
Sprint 003 Work and after
What does this Pull Request do?
Implements Collection Service with RDF aggregator.
Implements a pseudo monolithic PCDM Collection microservice implementation using ResourceServiceProvider
This services is built on one POST route, following the same logic as a ResourceService POST Route
Accepts and extra $uuid to be used as base resource for the PCDM collection
Some updates to composer and ResourceServiceProvider (not breaking)
What's new?
The collection Service accepts every RDF language/serialisation EasyRdf accepts via POST
Adds PCDM triples and UUID predicates (with an islandora experimental one) to given rdf body
If RDF body is empty it creates the resources too
Also:
How should Collection Service be tested?
Similar as the previous ResourceService, only POST Route needed
Basic (Manual testing, be sure to have a working composer environment)
$app['debug'] = truedefinition in index.phpAssuming you already have some containers in fedora 4 (with
nfo:uuid "7ef68f6f-72ab-4708-b0f4-8ec1f07cd580"for example) you can test the working route with a call like this (replace the uuid for one of yours)$ curl -XPOST http://localhost:8282/islandora/collection/7ef68f6f-72ab-4708-b0f4-8ec1f07cd580 -vThis will create a PCDM collection inside an existing Fedora4 Resource with
nfo:uuid "7ef68f6f-72ab-4708-b0f4-8ec1f07cd580"property.uuidrelated predicates added with a unique UUID V4 generated by the system (if not already present in rdf body):nfo:uuidandisl:hasURN(last one experimental)ldp:hasMemberRelation pcdm:hasMemberldp:insertedContentRelation ore:proxyFor$ curl -XPOST --data-binary "@body.rdf" -H "Content-Type:text/turtle" "http://localhost:8282/islandora/collection/7ef68f6f-72ab-4708-b0f4-8ec1f07cd580?tx=sometransactionid" -vAccepts this query argument optional(the stuff after the ?)
text/turtleit could be something like thisNote: if you don't have a CLAW vagrant environment working you can use(only for testing)
Note2: look at the response headers… there are a few nice ones!
*Since you won't have an indexer running, issue an insert data command to blaze graph to be able to test (you can use the same rdf response from fedora4 for a resource + nfo:uuid property to match using the web admin at http://localhost:9999/bigdata/#update)
Additional Notes:
This is a working but "test " Service that needs some cleanups, checks, (like the "use" statements) and can/will be moved in the future.
Thanks a lot!
Diego Pino Navarro
A dog & human friendly developer