-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Alternative Islandora ResourceService using providers #135
Conversation
Changed the one-file logic to full ServiceProvider and ControllerProvider. Allowed some external service injection/swapping possibilities for users, had to move the global middleware to specific middleware for each route to allow other silex apps to use and avoid all the transforms we do here
Added git VCS for chullo. No need to have it locally Removed a debug statement
$app['islandora.resourcecontroller'] = $app->share(function() use ($app) { | ||
return new \Islandora\ResourceService\Controller\ResourceController($app); | ||
}); | ||
if (!isset($app['twig'])) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this necessary, I thought the $app->share() took care of this type of logic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@whikloj, do you mean the "use" statements? I was unsure if putting there had any logic, but i thought it could be a good idea for code clarity/new users. I can remove it once all others had a look at the code
This is awesome @DiegoPino, I feel better knowing that I was close on some of my work. |
@whikloj, thanks, i really appreciate this. I'm pretty sure you where more than close. I did this pull also to serve as a base example on how to move any service to a provider, but i'm not sure if my implementation is the best, so i would love we could compare ways and more if you could make some pulls against this one! |
return; | ||
} | ||
//Not sure what the best "verbose" message is | ||
return new response(sprintf('Islandora Resource Service exception: %s / HTTP %d response', $e->getMessage(), $code), $code); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Capital R?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right!! moving to uppercase… sorry. Thanks for catching this @daniel-dgi and thanks @whikloj for explaining me what dani catched!! I can't even see my own code sometimes.
lazy php, more lazy diego
We have a monolithic project, git gets crazy in composer. Lets use path for now. Must be tuned during testing to local path. Before final merging will add real git.
Collection Service does nothing. But it allows to do anything. Whole remote serviceprovider/controllerprovider logic is working perfect, also UUID4 (and swappable V5 also). Test route transforms a post to collection into an internal GET to resource. Some debug error_log in place to see what is happening. Pretty happy!
Added new working code to help others to jump in. UUID generator (v4 and V5) service provider and a Collection Service Skeleton Thanks |
Awesome job @DiegoPino !! ResourceServiceProvider works 👍 Makes me wonder if these core services (Resource/Collection) should not be combined as one? Just combine the code so they share an index.php with the ResourceServiceProvider and CollectionService as sub-directories with the provider code. I'm good to merge this, but as it is Friday night I'll leave it for now incase there are any questions/concerns. I'll merge it tomorrow to give anyone a chance to test it themselves. Also, key point I missed. When pointing the collection service composer.json to the ResourceServiceProvider I forgot I had pulled the branch with a different name (sprint-3). If you do this, remember to change the |
Merging... we can move forward from here. |
Alternative Islandora ResourceService using providers
Thanks @whikloj for testing and merging! |
Sprint 003 Work
What does this Pull Request do?
Changed the one-file logic ResourceService silex app to a full ServiceProvider and ControllerProvider based app.
Allowed some external service injection/swapping possibilities for users, had to move the global middleware to specific middleware for each route to allow other silex apps to use and avoid
all the transforms we do here.
Twig was left as an independent registered Service to be swap-able.
Update:
What's new?
Simpler index.php file, loads the ServiceProvider and ControllerProvider as a unified multi inherited class to make editing easier. Route Mounting is now relative. All the global middleware was moved to each individual route (not pretty) to allow other apps to use this provider (Not sure how yet!) and avoid all their local routes to be parsed by this one.
How should ResourceServiceProvider be tested?
Same as the previous ResourceService, all basic REST methods are enabled, which means
POST, GET, PUT, DELETE, PATCH
Basic (Manual testing, be sure to have a working composer environment)
$app['debug'] = true
definition in index.phpAssuming you already have some containers in fedora (with uuid) you can test the working route with a call like this (replace the uuid for one of yours)
you have a child resource without uuid that is child of one with (lets say an image binary at /OBJ) you can issue this call
Also accepts this query arguments (the stuff after the ?)
Will fail if tx given and not a valid transactionid (404)
Will get fcr:metadata for any resource (e.g binary) if metadata=true
to can also get a resource that is direct child of root and has a simple path(no slashes) ( we double slash to avoid the uuid)
Note: if you don't have a CLAW vagrant environment working you can use(only for testing)
*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)
How should CollectionService be tested (stub route, will be expanded)?
1 Modify composer.json to match the path to your ResourceServiceProvider local location
2 run $ composer update
3 run $ php -S localhost:8282 -t src src/index.php this will use the php embed web server to test
curl -i -X POST http://localhost:8282/islandora/collection
will return the root resource using the ResourceService GET route. Not super cool, but watch the console!
TODO CollectionService
Some extra examples:
curl -i -X POST -H "Content-Type: image/jpg" -H "Slug:IMAGEN" --data-binary "@76.jpg" "http://localhost:8282/islandora/resource/7ef68f6f-72ab-4708-b0f4-8ec1f07cd580" -v
curl -i -X DELETE "http://localhost:8282/islandora/resource/7ef68f6f-72ab-4708-b0f4-8ec1f07cd580/IMAGEN?force=true" -v
IMAGEN
resource and ?force=true also gets rid of the tombstone. Without you keep the tombstonecurl -X PATCH -H "Content-type: application/sparql-update" --data-binary "@patch.rdf" http://localhost:8282/islandora/resource/7ef68f6f-72ab-4708-b0f4-8ec1f07cd580 -v -i
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. It was done to be able to use this routes inside the PCDM Collection Service and/or Object Service during development.
Thanks a lot!
Diego Pino Navarro
A dog & human friendly developer