Skip to content

Conversation

@rullzer
Copy link
Member

@rullzer rullzer commented Jul 20, 2016

This adds OCSMiddleware so apps that use OCSController have some control about the special way OCS works.

There is the generic OCSException and for now 3 speciailized exceptions for more type safety.

Also deprecated the old \OCP\API

Of course unit tests are added.

CC: @LukasReschke @nickvergessen @MorrisJobke @BernhardPosselt

@rullzer rullzer added enhancement 3. to review Waiting for reviews labels Jul 20, 2016
@rullzer rullzer added this to the Nextcloud Next milestone Jul 20, 2016
@mention-bot
Copy link

@rullzer, thanks for your PR! By analyzing the annotation information on this pull request, we identified @BernhardPosselt, @DeepDiver1975 and @nickvergessen to be potential reviewers

@rullzer rullzer mentioned this pull request Jul 20, 2016
5 tasks
@@ -0,0 +1,83 @@
<?php
/**

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty newline?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm I just ran the license script. Will fix.

@LukasReschke
Copy link
Member

I like it 👍

* @return OCSResponse
*/
public function afterException($controller, $methodName, \Exception $exception) {
if ($controller instanceof OCSController) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd probably combine the if with the next if. That way you nest one level less which makes it much more readable

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah fair enough. Leftover from when I did not have the generic Exception yet. Will fix.

@BernhardPosselt
Copy link
Member

Except for merging the two ifs I don't see anything that can be improved. If implemented 👍

rullzer added 3 commits July 20, 2016 20:03
* OCSException
* OCSBadRequestException
* OCSForbiddenException
* OCSNotFoundException
@rullzer
Copy link
Member Author

rullzer commented Jul 20, 2016

Comments fixed.
rebases
Lets wait for CI and then merge.

@MorrisJobke MorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jul 20, 2016
@rullzer rullzer merged commit 78cad69 into master Jul 20, 2016
@rullzer rullzer deleted the ocs-middleware branch July 20, 2016 19:04
R0Wi pushed a commit to R0Wi/server that referenced this pull request Nov 22, 2025
…ion-handling

fix(notifications): Notifier::prepare() threw \InvalidArgumentExcepti…
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release Ready to be released and/or waiting for tests to finish enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants