Spongeapi: Make ChangeServiceProviderEvent Cancellable

[justisr]

Without a cancellable implementation:

@Listener
public void serviceChange(ChangeServiceProviderEvent e) {
    if (!(e.getNewProvider() instanceof MyServiceImpl)) {
        Sponge.getServiceManager().setProvider(this, MyService.class, MyServiceImpl.INSTANCE);
    }
}

Service A wants their service provider to be the highest priority, so they listen for this event and then immediately change the service back to their own provider.
Service B also wants to be the highest priority, so they do the same thing and listen for this event and immediately change the service back to their own provider.
Both plugins are installed and they go back and forth swapping each other out until a stack overflow occurs.

With a cancellable implementation:

@Listener
public void serviceChange(ChangeServiceProviderEvent e) {
    if (!(e.getNewProvider() instanceof MyServiceImpl)) {
        e.setCancelled(true);
    }
}

Service A listens for this event, verifies that the new provider isn’t their own, and cancels the event.
Service B listens for this event, but their service wasn’t registered before service A, and so the service will forever be provided by A’s implementation.

If the implementing services were aware of each others’ potential existence and took it a step further to un-cancel a service change where the new provider was their own service, it would again come down to which listener was passed the event last which could be influenced by the order they set with their listener annotation, but it will never result in an infinite loop.

[ST-DDT]

Also, if both plugins use exactly this check then the the service provider won't be set at all...

[ST-DDT]

IMO, plugins shouldn't use this kind of "who is stronger" instead they should set the service once and then assume the correct one is registered unless they explicitly have support for pluginX's variant.

Logging a warning in case of an incompatible variant is probably helpful as well.

[dualspiral]

XY problem right here. You're suggesting a (bad) solution here for a problem that doesn't really exist.

In general, you're going to provide one plugin that supplies an implementation to one service. Economy and Permission plugins are good examples of that. If you install more than one of these types of plugins, then more fool you. You'd be told by one or the other plugin developers to remove one of them. In that regard, the problem becomes one of user error, rather than server/plugin error.

So, with that in mind, I'd argue the same logic replies. If the behaviour that occurs is due having two plugins implementing the same service, then one of the plugins should be removed as you're potentially implementing duplicate functionality.

Why do you think this is suddenly going to be a problem when it hasn't been historically?

[justisr]

Also, if both plugins use exactly this check then the the service provider won't be set at all...

That’s assuming that both plugins get loaded at the same time, but that’s never the case. One plugin gets loaded up, registers the service and its listeners. Second plugin gets loaded up attempts to register its service and gets cancelled, its listener is too late to cancel the initial registration. I believe the second code block would work fine.

XY problem right here. You're suggesting a (bad) solution here for a problem that doesn't really exist.

In general, you're going to provide one plugin that supplies an implementation to one service. Economy and Permission plugins are good examples of that. If you install more than one of these types of plugins, then more fool you. You'd be told by one or the other plugin developers to remove one of them. In that regard, the problem becomes one of user error, rather than server/plugin error.

So, with that in mind, I'd argue the same logic replies. If the behaviour that occurs is due having two plugins implementing the same service, then one of the plugins should be removed as you're potentially implementing duplicate functionality.

Why do you think this is suddenly going to be a problem when it hasn't been historically?

Spigot’s ServicesManager allows multiple services to be registered and used. I’m moving from that to Sponge, which only allows one service to be registered, and can’t help but feel it lacks the same flexibility. Especially considering Sponge determines which one gets registered based on plugin loading order, which basically comes down to alphabetical naming of the plugin. Completely arbitrary.

I’m attempting to accomplish two things:
1) Allow server owners to self-determine a priority for the plugin provided services they’re using rather than relying on load order.
2) Allow the server owner to choose to use multiple service implementations, since one may know how to handle input X but not Y, and the other may know how to handle Y but not X.

My first impulse was to base this on the existing ServiceManager and simply keep a second registry of all of the 3rd party services being provided, and then delegate appropriately.

If I can’t use the existing ServiceManger, I’ll have to essentially create my own version of a service manager that can satisfy the needs of a service with multiple running implementations, but that’s not ideal.

[ItsDoot]

As dual noted above (referring to the XY problem), it would help to explain the actual issue you're having, regarding needing multiple service implementations at once, in the case that it may be solved in a completely different manner; then we don't need to speak in abstract terms and can get right down to the real world use cases.

[dualspiral]

1. Allow server owners to self-determine a priority for the plugin provided services they’re using rather than relying on load order.

I can see that argument - we do something similar with commands where we say "hey, this alias must be registered with plugin X". I'd be okay with adding some sort of configuration to do this - that's heading into impl land though.

1. Allow the server owner to choose to use multiple service implementations, since one may know how to handle input X but not Y, and the other may know how to handle Y but not X.

I'd argue services should be as small as possible, and thus something like this should be two services. Your suggestion of input handing may not work in some scenarioes, for example, what if two economy plugins are installed and plugin A doesn't implement account deletion but plugin B does? In the case A takes precidence, would a delete call head to plugin B instead and perhaps do something unexpected? It probably shouldn't.

Why do you think this is suddenly going to be a problem when it hasn't been historically?

This is the crucial point you haven't answered. You've told me what Spigot does, but I don't have a good example to hand as to why this is useful to you. This is clearly related to your plugin development, so give us your example. Why is registration of multiple implementations of the same services of use to you?

[XakepSDK]

ChangeServiceProviderEvent event is made for other plugins to observe service changes. It's not indented to replace or cancel this event. If you want to enforce your service, just crash server with proper error message, admin should handle this.

[justisr]

ChangeServiceProviderEvent event is made for other plugins to observe service changes. It's not indented to replace or cancel this event. If you want to enforce your service, just crash server with proper error message, admin should handle this.
This would result in crashing the server every time it’s loaded up after a plugin that implements my service interface, but my service interface would be useless without those other plugins providing implementation.

As dual noted above (referring to the XY problem), it would help to explain the actual issue you're having, regarding needing multiple service implementations at once, in the case that it may be solved in a completely different manner; then we don't need to speak in abstract terms and can get right down to the real world use cases.
This is the crucial point you haven't answered. You've told me what Spigot does, but I don't have a good example to hand as to why this is useful to you. This is clearly related to your plugin development, so give us your example. Why is registration of multiple implementations of the same services of use to you?

Fair enough. When it comes to a large projects though, I’ve found that specifying a use case often leads to resolutions being discussed that would apply only to that use case and not to others that are related and could have benefited from it being discussed and resolved in more abstract terms.

I’m making an "ItemBroker" service interface for plugins like "HarvesterHoes", "CropHoppers", "Excavators" or any of the dozens of plugins that automate the sale of item drops, or offer additional buy/sell functionality through click events, custom inventories, etc. to pass off their purchases/sales to.
Any server that has these plugins installed will have at least one shop plugin installed. It might have a sign shop installed, a chest shop installed, a GUIShop installed, it might have all of them. However, they each already have prices set for items. These extended plugins will want to pass the target ItemStack being bought/sold to whichever shop plugin is installed, and have it buy/sell it on their behalf.

1. Allow server owners to self-determine a priority for the plugin provided services they’re using rather than relying on load order.

This was the true purpose of the cancellable implementation I wanted. This way, my plugin, the one providing the service interface as well as the configuration, could enforce the priority that the server owner has set.

1. Allow the server owner to choose to use multiple service implementations, since one may know how to handle input X but not Y, and the other may know how to handle Y but not X.

This was more of a happy side effect of having that control. If the highest priority plugin’s transaction response returns an equivalent to "not implemented", perhaps due to it receiving a custom item that it’s not designed to handle, it can pass that ItemStack off to the next plugin in line until one of them knows how to handle the ItemStack, and only if none of them do, throw an error.

[dualspiral]

Okay, sorry, never got back to you on this, just saw it and wanted to explain some of the thinking for 1.14.x and how it actually relates to this issue, and ultimately why it's being closed.

You may have seen in #1dot14updates in our Discord guild from a discussion @Zidane and I had that we're actually looking at moving away from having our own service provider and using the Java SPI. Obviously this won't change for API 7.x, but the beauty of the SPI is that it's not linked to Sponge at all and so you can use it if you wish. A big thing that you seem to want is multiple implementations of a service - this will do as you wish, allowing you to control who gets what. All you'd probably want to do is register your own service in the Sponge service manager as an external API (or maybe not, who knows?)

A further point that came up in the Discord channel as well as above is the point that admins should control the service load order, not plugins. While you suggest that's your use case (and I can understand that!), it might not be others' - it's still putting service loading under a plugin's control and that is not a good thing.

Regardless, thank you for explaining what you were looking for and sorry it took so long to come back to you with this.