Feature/http api new approach #348

Jan-Schuppik · 2025-08-22T13:51:53Z

Initial setup for the new approach of the http-api with openapi description.

Resolves:

require:

nilmerg

This review covers only the early parts of routing and dispatching.

run.php

application/controllers/ApiController.php

library/Notifications/Api/V1/ApiV1.php

nilmerg · 2025-08-26T09:14:23Z

Oh, and I got the postgres tests working in my environment. Please get back to me for this.

test/php/application/controllers/ApiV1ChannelsTest.php

test/php/application/controllers/ApiV1ContactsTest.php

library/Notifications/Api/ApiCore.php

library/Notifications/Api/V1/ApiV1.php

library/Notifications/Api/V1/Channels.php

library/Notifications/Api/V1/Contacts.php

sukhwinder33445

I have looked at channels and contact tests so far, next I will look at contactgroup tests.

library/Notifications/Api/V1/ApiV1.php

test/php/application/controllers/ApiV1ContactsTest.php

test/php/application/controllers/ApiV1ChannelsTest.php

test/php/application/controllers/ApiV1ContactGroupsTest.php

library/Notifications/Test/BaseApiV1TestCase.php

test/php/application/controllers/ApiV1ContactGroupsTest.php

library/Notifications/Api/V1/ContactGroups.php

library/Notifications/Api/ApiCore.php

sukhwinder33445

Please add following changes as well:

The PHPDoc for the method must contain a newline between different annotations, i.e. @param, @return.
Update method PHPDoc if required.
Make the getPlural() method protected.
Remove the resolved todos.

library/Notifications/Api/ApiCore.php

library/Notifications/Api/V1/Contacts.php

application/controllers/ApiController.php

library/Notifications/Api/V1/OpenApi.php

application/controllers/ApiController.php

library/Notifications/Api/ApiCore.php

application/controllers/ApiController.php

library/Notifications/Api/ApiCore.php

library/Notifications/Api/V1/ApiV1.php

library/Notifications/Api/V1/Channels.php

library/Notifications/Api/V1/ApiV1.php

library/Notifications/Api/V1/Contacts.php

library/Notifications/Api/V1/ApiV1.php

nilmerg

Let's discuss how we handle the openapi description and the usage of Swagger, as there are three possible solutions for me:

Caching (properly, not the way it's right now ;) )
CI (as your todo suggests)
Static (it's a versioned api after all)

application/controllers/ApiController.php

nilmerg · 2025-10-08T11:46:24Z

application/controllers/ApiController.php

Missing license header. It seems, not only this file is missing it, please make sure all PHP files have one.

nilmerg

Don't worry, I didn't take a look at the validation/middleware stuff again 😜

application/controllers/ApiController.php

library/Notifications/Common/PsrLogger.php

library/Notifications/Api/OpenApiDescriptionElement/Response/SuccessDataResponse.php

library/Notifications/Api/Util/BinaryUuidConverter.php

nilmerg · 2025-10-21T12:25:08Z

Adjusted and extended the tests quite a bit. The failing tests uncover the following problems:

Invalid default channel or user UUIDs will result in a HTTP 400, whereas other errors like invalid group UUIDs or invalid address types result in a HTTP 422
An unknown channel UUID yields an incorrect error message
Changing group memberships through either the contacts or contact-groups endpoints is broken (as you've identified by yourself already)

nilmerg · 2025-10-22T07:04:44Z

library/Notifications/Api/Middleware/MiddlewarePipeline.php

+        foreach ($middlewares as $middleware) {
+            try {
+                $this->pipe($middleware);
+            } catch (\Exception $e) {


If you really must throw your own exception, catch TypeError instead

nilmerg · 2025-10-22T07:09:53Z

doc/20-REST-API.md

@@ -0,0 +1,31 @@
+# REST API
+
+The Icinga Notifications module provides a REST API that allows you to manage notification-related resources programmatically.


~~The Icinga Notifications module~~ Icinga Notifications Web

nilmerg · 2025-10-22T07:12:37Z

doc/20-REST-API.md

+
+This API enables easy integration with external tools, automation workflows, and configuration management systems.
+
+---


These look weird, how did you come up using them? I'd rather remove them

nilmerg · 2025-10-22T07:16:39Z

doc/20-REST-API.md

+
+## API Versioning
+
+The Notifications API follows a **versioned** structure to ensure backward compatibility and predictable upgrades.


Before

this API

This API

And now

Notifications API

Later

… API … reference

Please stick to a single version, so remove Notifications here.

nilmerg · 2025-10-22T07:31:42Z

library/Notifications/Api/V1/ContactGroups.php

+use Icinga\Module\Notifications\Model\Contactgroup;
 use Icinga\Module\Notifications\Model\Rotation;
 use Icinga\Module\Notifications\Model\RotationMember;
 use Icinga\Module\Notifications\Model\RuleEscalationRecipient;


Just noticed. These must not be used! The ORM must be bypassed.

nilmerg · 2025-10-22T07:34:07Z

library/Notifications/Api/V1/Contacts.php

 use Icinga\Exception\Json\JsonEncodeException;
 use Icinga\Module\Notifications\Api\EndpointInterface;
 use Icinga\Module\Notifications\Api\Exception\InvalidFilterParameterException;
+use Icinga\Module\Notifications\Model\Contact;


Same here, plus the ones below, no ORM usage

nilmerg · 2025-10-22T12:35:58Z

library/Notifications/Api/Middleware/RoutingMiddleware.php

+            $request
+                ->withAttribute('version', ucfirst($version))
+                ->withAttribute('endpoint', ucfirst($endpoint))
+                ->withAttribute('identifier', $identifier)


MySQL uses a normal char(36), whereas PostgreSQL uses a native type, but only the native type is case-insensitive, so we should lowercase here. I think it's fine to do it here and assume everywhere else that it's already lowercase.

Oh and, not sure what the validation middleware accepts, but we have to also make sure the UUID is exactly 36 chars. (No curly braces)

Jan-Schuppik requested a review from nilmerg August 22, 2025 13:51

Jan-Schuppik self-assigned this Aug 22, 2025

cla-bot bot added the cla/signed CLA is signed by all contributors of a PR label Aug 22, 2025

Jan-Schuppik force-pushed the feature/http-api-new-approach branch from 9a3a6aa to aa2b625 Compare August 25, 2025 12:48

nilmerg requested changes Aug 26, 2025

View reviewed changes

This was referenced Sep 3, 2025

Add column external_uuid to contact/contactgroup table Icinga/icinga-notifications#216

Open

HTTP API to configure contacts and contactgroups #199

Closed

Jan-Schuppik force-pushed the feature/http-api-new-approach branch 2 times, most recently from 1705125 to fe816f8 Compare September 3, 2025 10:58

Jan-Schuppik mentioned this pull request Sep 5, 2025

Feature/notifications api Icinga/icinga-php-thirdparty#58

Merged

sukhwinder33445 reviewed Sep 8, 2025

View reviewed changes

sukhwinder33445 reviewed Sep 9, 2025

View reviewed changes

sukhwinder33445 reviewed Sep 11, 2025

View reviewed changes

Jan-Schuppik requested a review from sukhwinder33445 September 15, 2025 06:22

sukhwinder33445 reviewed Sep 18, 2025

View reviewed changes

Jan-Schuppik requested a review from sukhwinder33445 September 19, 2025 07:01

sukhwinder33445 reviewed Sep 19, 2025

View reviewed changes

library/Notifications/Api/ApiCore.php Outdated Show resolved Hide resolved