RFC: Split OCP into Consumable and Implementable to clarify intentions

[nickvergessen]

What?

1. We have various interfaces where it is clear that apps are meant to implement them: - OCP\Notification\IApp
   • OCP\Notification\INotifier
2. We have various interfaces where it is clear that apps can consume them, but it is not intended to get implemented by an app itself:
   • OCP\Notification\IManager
   • OCP\Notification\INotification (should only be created with OCP\Notification\IManager::createNotification())

From my POV it would be good to distingiush them in a way, as they come with different implications.

• OCP\Notification\IManager is only meant to be consumsed, so we can add new methods and it could be considered non breaking as the only implementation is meant to be the one in OC\Notification\Manager. We can add return types and nothing explodes
• For OCP\Notification\IApp it's the opposite, basically extending it will break all existing implementations. Similar to OCP\Dashboard\IWidget back then, new functionality needs to be brought in with a second interface IAPIWidgetV2, etc.

• My idea would be to introduce attributes which clarify the intention of the publishing, similar to Implement an OCP API for Context Chat server#52852 (comment)
  Something along the lines:
  • OCP\AppFramework\Consumable(since: MajorMinorServer, ?removal: MajorMinorServer)
  • OCP\AppFramework\Implementable(since: MajorMinorServer, ?removal: MajorMinorServer)

Why?

• Allows faster progress on some APIs (
• Clarifies intention
• Should make it clear that things like Guests app RestrictionManager which replaces OCP\INavigationManager and OCP\Settings\IManager are not a good idea because both should be "Consumable"-only

Optional Part2:

To help with the last "Why" we could also allow apps to mark themselves as an exception to such a restriction, which means we "allow it when we are aware" and API can be extended when either adjusting the exception or clarifying a way with the maintainer of the exception how it can be "non-breaking" in the update experience.

Sample:

<?php

declare(strict_types=1);
/**
 * SPDX-FileCopyright…
 */
namespace OCP;


#[OCP\AppFramework\Consumable(since: '6.0.0')]
#[OCP\AppFramework\ExceptionalImplementation(app: 'guests', class: \OCA\Guests\FilteredNavigationManager::class)]
interface INavigationManager {

[blizzz]

👍 for it basically document's lived practice

[come-nc]

I love the idea.

OCP\AppFramework\Implementable(since: MajorMinorServer, ?deprecated: MajorMinorServer, ?removal: MajorMinorServer)

I am against having deprecated in there, it should be its own attribute.
We should even directly use https://www.php.net/manual/en/class.deprecated.php

And removal, I do not understand what it’s for. If the API is removed the attribute is not there anymore?

But apart from this bikeshedding, please go for it!

[come-nc]

I cannot find a specific PR or issue, but we tried to include this info before like this in some places:
https://github.com/nextcloud/server/blob/47c0786a3f316f00d3de4e215825418cd4c39718/lib/public/Lock/ILockingProvider.php#L14

[nickvergessen]

I am against having deprecated in there, it should be its own attribute.
We should even directly use https://www.php.net/manual/en/class.deprecated.php

Totally right.

And removal, I do not understand what it’s for. If the API is removed the attribute is not there anymore?

My idea would be to add this when it's deprecated so that the attribute could be used by apps as well (steaming from nextcloud/server#52852 (comment)) to have a clear/dedicated timeline for the removal, if the app would follow another deprecation period than the 3y of OCP.

[nickvergessen]

• Draft PR is available in feat(OCP): Consumable vs. Implementable public API server#53072
• I additionally added Throwable and Dispatchable for better semantics on exceptions and events
  • Should I also add Catchable and Listenable?
• I adjusted the OcpSinceChecker to require at least one of the new Attributes (limited to Notification and a single Enum for demo purpose)
• The attribute are rather neutral (only requiring a since: 'version' which allows apps to apply the attributes to their APIs as well in the future