microsoft / AL

Home of the Dynamics 365 Business Central AL Language extension for Visual Studio Code. Used to track issues regarding the latest version of the AL compiler and developer tools available in the Visual Studio Code Marketplace or as part of the AL Developer Preview builds for Dynamics 365 Business Central.
MIT License
733 stars 243 forks source link

Opportunistic AL subscribers: SkipOnMissingPublisher [AL0185, AL0280] #1852

Open XVII opened 6 years ago

XVII commented 6 years ago

I have an issue where the compiler fails with AL0280 & AL0185 due to missing pages and publishers in the EventSubscriber attribute.

AL0280: Page 'x' is missing
AL0185: The event 'x' is not found in the target

(EDIT: I've clarified the suggestion below based on feedback)

SkipOnMissingPublisher

A SkipOnMissingPublisher property in the EventSubscriber attribute would sit nicely against SkipOnMissingLicense and SkipOnMissingPermission.

Justification

While the compile errors are valid in most situation (as it highlights a disconnect and invalid code), for the EventSubscriber attribute there are scenarios, such as conditional integration to other extensions, where it shouldn't raise a compilation error.

Example where Page 70000000 might not exist, or OnOpenPage might not exist as a publisher

[EventSubscriber(ObjectType::Page, Page::70000000, 'OnOpenPage', '', false, false)]
local procedure ExamplePage_OnOpenPage(var Rec : Record Example);
...

In the example above, if 70000000 was another ISV solution, then the Extension would fail to compile and publish due to the missing page.

What about dependencies?

Adding a dependency is not viable as it binds the extensions together on what should otherwise be opportunistic. Dependencies also require an entire prerequisite extension to be installed that otherwise shouldn't have to be.

C/SIDE Behaviour

In the C/SIDE environment this would have compiled without error. The classic development environment lets you subscribe to non-existent publishers (by writing the page ID) at compile time, then during runtime, if the publisher exists, it's wired up via Event Subscriptions.

Discovery Event Pattern

This also affects the discovery event pattern. Let's theorize and say a company came up with a "Assisted Setup" (not the standard one). Another app couldn't register in that Assisted Setup without listing it as a strict dependency. It would be nice to be able to "optionally" subscribe and only add entries if that Publisher existed.

Version

This occurs in AL Language v0.14.17461 and v0.15.18771.

kalberes commented 6 years ago

I do not think the C/SIDE scenario is the best choice here. The reason why you see this in C/SIDE is a legacy issue. Meaning that we wanted the least impact on potentially breaking the legacy compiler. From our standpoint we want to "protect" our server from doing potentially expensive failing runtime checks, when we have a compiler to validate application object validity. Of course the best would be if we would have proper DI patterns here. That is something we have in our backlogs but it is for the future for now.
I will forward this issue though.

XVII commented 6 years ago

The application isn't invalid because of this; it's just leveraging events and decoupling two plugins that potentially integrate.

A few follow up questions;

- If a user installed this extension that was missing a publisher, would it fail to activate? EDIT: Yes, it does. The next thing I'm going to try is install another extension that exposes those objects (a dummy object) then uninstall it.

- If not, if I can create dummy symbols to fool the compiler? This seems unnecessary. EDIT: Yes, the symbols make it compile fine, but publishing re-validates the rules. The dependency system also prevents related extensions from being uninstalled.

- Couldn't/shouldn't the Event Subscription checks be done on activate? e.g. if the publisher doesn't exist, then it should not activate the Event Subscription.

- What DI pattern would you expect to use? I haven't used optional dependencies in that scenario. Wouldn't an implementation like that require runtime Event Subscription checks also? Same with reflection?

XVII commented 6 years ago

Another thought I had was to introduce a "shim" of sorts -- to try and fill the publisher gaps for those without the additional functionality.

The problem here is that the shim must be marked as a dependency before it'll recognize the page (the new extension doesn't see it and still says error AL0185: Page 'x' is missing )... but listing it as a dependency breaks installation on a system that has the real, non-shim version.

Shims aside...

I think I'll end up with two versions of the same app -- which is an annoying maintenance issue. I'd prefer one version that is smart enough to work with other add-ins without being a strict dependency. Lost a really powerful feature with the move to AL.

Would a SkipOnMissingPublisher property on the attribute be appropriate? Just like SkipOnMissingLicense and SkipOnMissingPermission...

kalberes commented 6 years ago

Yes i think the SkipOnMissingPublisher is a good idea. I will accept it as a bug for now.

XVII commented 6 years ago

Thanks for the consideration. I'll update the OP to clarify the suggestion and summarise the change in behaviour.

kalberes commented 6 years ago

I have talked to the team and this issue was a conscious design decision. I will create a backlog item to solve this and other similar scenarios where C/SIDE allowed id based weak references to be added.

XVII commented 6 years ago

Some more thought regarding the name of this property/attribute -- "SkipOnInvalidPublisher" would probably be more appropriate.

This would cover both scenarios of:

  1. Subscribing to an event that doesn't have a related publisher.
  2. Having a signature mismatch between publisher and subscriber.
kalberes commented 6 years ago

We will probably add two different subscriber syntaxes for strongly typed and weakly typed references. There are people who hate that C/SIDE behaves like that. See issue #1972

MAC-Christian commented 6 years ago

I would appreciate to have this possibility. In my company we are breaking our ISV solution into smaller modules. So as a result we have some modules which could run on NAV-Cronus as a stand-alone App. With the behaviour of subscriber events on missing publishers in C/AL, we can do this without problems, as we use Publisher-Events when we identify an interface/communication between two modules. In AL we have to add dependencies (and create a strong binding between modules...that is what we are trying to avoid) or as ShadowXVII wrotes have two different versions.

ThomasBrendel commented 5 years ago

How long do you think will it last from your backlog into real programming? Because its vital to know how we can design our possible extensions beforehand.

If we can write inactive subscribers we mustn't design "dependency extensions" and we reduce the amount of those "intermittend extensions". We have a "huge" application and want it to capsule and cut into pieces. But without "inactive subscribers" we assume to have maybe 50 plus "dependency extensions"!

I can hear the cries of consultants and customers just now, when they have to install those 50+ extensions .... not mention the possible support afterwards:-)

So it is vital to find a solution for that problem.

dannoe commented 3 years ago

Any updates on this?