Remove apps that will go to market from shipped.json

[PVince81]

Description

Makes it possible to uninstall them.

Related Issue

#27983

Motivation and Context

How Has This Been Tested?

• TEST: setup works
• TEST: apps from the list still work
• TEST: occ ugprade doesn't complain, still works (no auto-disable anyway)
• ❌ TEST: removed apps can be uninstalled now Cannot uninstall LDAP app via market app after upgradid from oC 9.1.5 #28001

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.

I don't have full understanding of the extent of the "shipped" flag. Grepping for isShipped seems to show a lot of locations, even some related to API responses.

@tomneedham @DeepDiver1975 @davitol

[davitol]

Tested and WFM 👍

[PVince81]

Thanks. Don't merge yet, there could be consequences due to the weird OCS code that rely on this flag.

Needs to be evaluated

[tomneedham]

So grepping all of these apps to show what OCS routes they might register:

./activity/appinfo/routes.php://API::register(
./activity/appinfo/routes.php://API::register(
./activity/appinfo/routes.php:API::register(
 ./notifications/appinfo/routes.php:\OCP\API::register(
./notifications/appinfo/routes.php:\OCP\API::register(
./notifications/appinfo/routes.php:\OCP\API::register(
./notifications/tests/Integration/app/appinfo/routes.php:\OCP\API::register(
./notifications/tests/Integration/app/appinfo/routes.php:\OCP\API::register(
./sharepoint/appinfo/routes.php:OC_API::register('get', '/apps/sharepoint/api/v1/mounts',

[tomneedham]

My only concern at the moment is if apps are enhancing the notification or activity apis - looking for usages...

[PVince81]

AFAIK none of our apps are expecting more than a single response block.

Does it make a difference is shipped is there or not ? Does it influence the order or name of the response block ?

[DeepDiver1975]

Kill market as well

[PVince81]

this will allow uninstallling the market app, (P)roceed, (A)bort ?

[DeepDiver1975]

If somebody wants to....

[PVince81]

My only concern at the moment is if apps are enhancing the notification or activity apis - looking for usages...

Enhancing ? How ?

[PVince81]

and why

[tomneedham]

Enhancing ? How ?

well by latching onto the same route and altering / changing the response in some weird way

and why

no idea

[PVince81]

removed "market".

@tomneedham is checking whether it's really possible to extend/overwrite the endpoints in 10.0. Because from what I remember we removed this possibility a long time ago when unifying the APIs. (something about integrating with app framework I think)

[tomneedham]

Just checked with 10.0.1 => You can still 'extend' existing routes by registering for the same route and returning data - the responses are then 'merged'. This is done using array_merge with priority given to shipped apps. Collisions would only occur when using the same keys in the responses.

[PVince81]

@tomneedham thanks. This kind of weird overriding of routes isn't officially supported. I thought we properly removed this already. Might need more work for this.

This means at this point we will remove the apps anyway from shipped.json. The likeliness of someone overwriting routes like this is anyway minimal.

Waiting for CI then merge

[PVince81]

@davitol did you test the checkboxes I posted above ? If yes, please tick

[PVince81]

Test passed: https://jenkins.owncloud.org/job/owncloud-core/job/core/job/PR-27985/

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.