Standardise how we create API/FormSchema endpoint in the CMS

[maxime-rainville]

Story

As a CMS developer, I want a clear and standardised way to add endpoints in the CMS.

Context

We just seem to be peppering new API endpoints throughout the CMS whenever we need them without thinking things through. This is creating technical debt.

• AssetAdmin defines a bunch of its own endpoint on its own controller
• CampaignAdmin defines a bunch of endpoints as well
• Content-review adds an extension to LeftAndMain
• [Elemental as an ElementalAreaController] that extends CMSMain
• AdminRegistrationController creates an hidden LeftAndMain controller.
• LoginSessionController extends Controller even if it's only meant to be used in admin.
• ShareDraftContentControllerExtension creates a MakeShareDraftLink action but because it's added on the Controller class the action is added on every single controller
• UserDefinedFormAdmin extends LeftAndMain
• HistoryViewerController extends left and main so we can create a few form schema endpoints.
• We're always re-inventing the JSONResponse method/class.

Acceptance criteria

• There's a standardised and documented way of creating new API endpoints in the CMS. This accounts for:
  • Validating access
  • Allowing third party module to customise and add their own endpoint
  • Error handling
  • Communicating the endpoints to the front end
• There's a standardised and documented way of creating new FormSchema endpoints in the CMS.
  • This accounts for form validation
  • This accounts for communicating success to the front end (e.g. Displaying success message)
  • Extra context about the success of the form submission can be communicated to the parent JS process
• There's a standardised and documented way of creating JSON response.

Related cards

• Implement validation method for inline editable form silverstripe-elemental#329
• Validation of standard link types silverstripe-linkfield#17
• Standardise how we create API/FormSchema endpoint in the CMS #1589

[maxime-rainville]

I've created one card for now. But this will probably need to be split in a few cards to make sense.

Maybe one of the way we do this is fine and we just need to agree and document it.

I'd be keen to explore what helper classes we can put in place to simplify the process beyond that. (Creating a JSONReponse class seems like an obvious win)

[emteknetnz]

I think I'd be more in favour of a jsonResponse($data): HTTPRequest style method rather creating than a JSONReponse class. Adding a class means there's now an extra types of thing that we need to handle that can be returned from controller actions. Currently the normal things returned are HTTPResponse's and strings. A jsonResponse() style method could look something like this (untested)

private const DEFAULT_JSON_OPTIONS = JSON_UNESCAPED_SLASHES + JSON_UNESCAPED_UNICODE;

protected function jsonResponse(array $data, $jsonOptions = self::DEFAULT_JSON_OPTIONS): HTTPResponse
{
    $response = $this->getResponse();
    $body = $json_encode($data, $jsonOptions);
    // if json error, do something to handle it
    $response
        ->addHeader('Content-type', 'application/json')
        ->setBody($body);
    $this->invokeWithExtensions('updateJsonResponse', $response);
    return $response;
}

[emteknetnz]

Seems like there's a few patterns here

1. extends LeftAndMain + $url_segment (seems good)

• e.g. MyController extends LeftAndMain ... private static $url_segment = 'mymodule'; will setup routing to your controller for /admin/mymodule
• Note this includes classes that extends CMSMain (which itself extends LeftAndMain)
• This seems like the best method to me and is what the ModelAdmin's use
• Use must be logged into the CMS in order to access the endpoints
• You can do nested endpoints by creating new controllers and just having a nested $url_segment e.g. CMSPageSettingsController has a $url_segment of "pages/settings"
• Gets added to AdminRootController::rules();

2. extends LeftAndMain + $url_handler (seems good)

• The same as 1), though also use private static $url_handler
• This is what the asset-admin controller is doing
• Seems very useful for nesting endpoints in a single file. Also allows restricting to particular HTTP methods.
• Nesting endpoints here has a downside, where the "//$Action/$ID/$OtherID" params available via $this->getRequest()->params() are "offset" e.g. when upoading a file to AssetAdmin::apiCreateFile will have Action: "api" and ID: "createFile" when I'd expect `Action: "createFile". It's not a huge deal and can be easily worked around.

3. extends Controller + LeftAndMainExtension + action that returns a MyController

• A jankier method that puts an $allowed_action + method directly on to LeftAndMain via an extension instead of using $url_segment. The method returns new MyController(). Routing magically works from that point.
• $this->getRequest()->params() are a little off in my testing in that $ID + $ExtraID will be set to whatever $Action is if they're unset. This seems like a bug.
• Means that your Controller's API is "cleaner" than extending LeftAndMain, though the routing is confusing for maintainers IMO.
• Won't get added to AdminRootController::rules();

4. extends Controller, routes.yml and Member check (hopefully)

• Define the path to the endpoint via routes.yml.
• This is the worst approach for /admin/foo endpoints because of the need to explicitly check if the member is logged into the CMS. Mistakes may happen.
• Won't get added to AdminRootController::rules();

5. Other

• Things like ShareDraftContentControllerExtension - oh dear

[GuySartorelli]

Ultimately I'd like to see a controller higher up the class hierarchy than LeftAndMain that handles admin permissions but doesn't make it a "Left" or a "Main"... but that's a CMS 6 concern. For now, some combination of the first two options seems preferred as far as how to route a controller that's used exclusively in the CMS.

[emteknetnz]

Yeah the logical thing is probably AdminController extends Controller which supports the $url_segment / $url_handler config fields, and automatically creates paths under /admin and requires you to be authenticated to get to them. We could at least create it in CMS 5 for anything new, though anything existing will need to stay extending LeftAndMain / CMSMain or whatever it's doing right now until CMS 6

[maxime-rainville]

I think I'd be more in favour of a jsonResponse($data): HTTPRequest style method rather creating than a JSONReponse class. Adding a class means there's now an extra types of thing that we need to handle that can be returned from controller actions. Currently the normal things returned are HTTPResponse's and strings.

JSONReponse would extend HTTPResponse. So you would still be returning an HTTPResponse, so no change would be needed to any controller.

That's also the approach Symfony has taken.

[emteknetnz]

As mentioned in meeting, JSONReponse wouldn't work great with when you go $this->getResponse() in a controller as getResponse() returns an HTTPResponse, so using instantiating a new JSONReponse would require copying over everything from the HTTPReponse to the JSONResponse

[maxime-rainville]

Just notice that campaign Admin seems to have form schema validation working and is using POST to submit data back. That might be the closest example we have to what Form Schema was meant to do.

campaign-2023-10-28_20.51.07.webm

[emteknetnz]

I'm really starting to wonder if the idea of "standardise/centralise" is something that's even viable or desirable.

It seems that a long time ago Silverstripe adopted a philosophy of "passing control" from one object to another to handle requests - for instance read the class docblock for RequestHandler - https://github.com/silverstripe/silverstripe-framework/blob/5/src/Control/RequestHandler.php#L19

The original philosophy seems it was built around the idea of having a series of nested RequestHandlers that handle their own little bit of the URL, rather than more centralised routing method

For instance /admin/<Controller>/<Form>/field/<FormField>/<FormAction>/<Param> - example given in RequestHandler is /admin/crm/SearchForm/field/Groups/treesegment/36

Controller and FormField both extend RequestHandler and Form implements HasRequestHandler - i.e. everything handles requests.

The routing for the above will (as I understand it, the exact details don't matter)

• /admin will route to AdminRootController
• /crm will return a LeftAndMain sublcass which is found via AdminRootController looking for LeftAndMain sublcass that has a $url_segment 'crm', let's call it CRMAdmin. Control is passed to CRMAdmin.
• /SearchForm is probably be the method CRMAdmin::SearchForm() which will probably also be registered on CRMAdmin.allowed_actions and will return a Form object. Control is then passed to the Form.
• /field/Groups will return a TreeMultiselectField, which is found via Form::getRequestHandler()->handleField('Groups'). Control passed to the TreeDropdownField
• /treesegment/36 is the result of TreeMultiselectField::treesegment(36) (note this method no longer exists, it's just the example in RequestHandler)

My point here is that Silverstripe CMS has for a long time had a very non centralised routing system and instead uses a nested routing system. This is unfortunately a confusing system that, while I'm sure the conventions made intuitive sense to the original designers of this, years later it now basically requires the use of a debugger to work out how something got routed. The nested routing does not lend itself well to any attempts at centralisation. Any dreams of a single page that shows all of the routes seems very far away.

While we could probably do something to centralise things e.g. make it so that you can send treedropdown XHR requests to /admin/api/TreeDropdownField/57, we're then introducing a new way to do the same thing which would just making the the Silverstripe problem of multiple ways to do the same thing worse

[maxime-rainville]

Some documentation has been written, but I'm not confident it meets the ACs of this cards.

I'm moving this card to the CMS 5.3 milestone.