RFC: Decorator Pattern

What is this?

Approved by:

Reality?

We have several hidden objects in OBS code base that are spread around several classes. Let me explain:

Open NotificationNotifiableLinkComponent class. Do you see the that big case? We're doing different things depending of the event_type of the notification, that means we have several subtypes of notifications.

We do the same again some lines below. For this type we do this, for that type we do that.

And we do the same again.

Ok. Those code pieces are part of the implementation of a notification for an event.

Consequences?

Every time we need to implement a new notification, we need to change several files. We need to add a description in one file, add the link to the notified thing in another, then the avatar in another one, then the excerpt in yet another one, etc.

You need to remember how many files do you have to change to implement one thing.

Future?

Wouldn't be easier to just open a new file to implement a new feature? Just one file.

And more important, you don't need to change existing code (remember the Open-Closed principle).

Alright, then we move all the logic for one event type into one class. What about moving it into the event itself? Okay, that could work, each one of the events would know how to create a notification. But we have a problem: the event is now having code related to the rendering of the notifications.

What's better is to use the Decorator pattern to dynamically add more features to the Notification model without polluting the model with rendering related code.

So, how can we do that?

1. We move all specific code regarding a notification type into a decorator class. Now that object has all the code related to rendering the specific notification.
2. Now, the Notification knows which type it is, so it can dynamically choose the appropriate decorator object.

So, in the end we have an object (the Decorator) that knows how to render a notification type, the Notification is free of code regarding rendering, the Component is simpler because it does not know how to render all the notifications types, it just uses one decorator.

Here it is a PR demonstrating the implementation of a Notification decorator and it's usage.

More Opportunities?

There are some other places like this where we could use the Decorator Pattern to refactor existing code:

• Abstracting out the differences between Gitlab, Github and Gitea
• Refactoring the actions we perform when detecting the different SCM's events. For example, what we do when we receive a pull request is scattered between SCMWebhookEventValidator, SCMWebhook, WorkflowRun, Workflow, Workflow::Step::TriggerServices and the TriggerControllerService::SCMExtractor