Add design documentation for extension mechanism#9
Conversation
Introduces the Technical Design Specification for decoupling domain-specific logic (Energy, Protocols, UI) from the platform Core. Closes openremote/openremote#2327
|
|
||
| ### What is an Extension? | ||
|
|
||
| Technically, an extension is a versioned artifact (such as a JAR file) that the OpenRemote Manager can discover, load, and initialize at runtime. |
There was a problem hiding this comment.
"discover, load, and initialize at runtime" the wording was a bit confusing for me at this stage but it's clearly explained later. Here however I was afraid of going towards dynamic loading and OSGi type requirements.
ebariaux
left a comment
ebariaux
left a comment
There was a problem hiding this comment.
That's a great start, I think you covered quite a bit of ground and it's good to have things explicitly described and written down.
I made quite a few comments that I think will be subjective. My proposal would be that once there's been feedback from a few reviewers, we organise a session to go over all the remarks and align on a direction to proceed with.
| * **Connectivity**: Protocol agents such as Artnet, ChirpStack, KNX, and ZWave. | ||
| * **Logic & Rules**: Support for Groovy/Flow rules, Container Services (Forecasting, Simulators), Rule languages (similar to JavaScript and JSON). | ||
| * **Identity**: Pluggable Identity providers (similar to the Keycloak and Basic IdPs). | ||
| * **Storage**: Infrastructure drivers for Operational data (JDBC) or Historical data (TimescaleDB). |
There was a problem hiding this comment.
I'm not sure we want to go that far with extensions but it doesn't really hurt to leave it as a vision.
Wondering then how to manage the link with potential other requirements for other components in the whole infra (e.g. we need a Keycloak container).
| ### Namespace Convention | ||
|
|
||
| To ensure architectural clarity and prevent classpath collisions, all extensions must adopt the following base package: | ||
| `org.openremote.extension.{extension-name}` |
There was a problem hiding this comment.
Wondering if at some point we want to move to io.openremote ?
If so, might be a good idea to do it before making anything public about extensions.
| ### Testing Strategy (Spock & Groovy) | ||
|
|
||
| To ensure reliability, extensions should include a comprehensive test suite. | ||
| Following the OpenRemote Core standard, we use the Spock Framework with Groovy for integration and unit testing. |
There was a problem hiding this comment.
Is this really standardized ? I have been introducing several "pure Java JUnit" unit tests as I feel there's a big overhead to using Spock for simple unit test.
There's also the fact that in the core repo, Spock tests are in their dedicated gradle project/module and not within the project where the code under test resides. This is also one reason I prefer JUnit tests along side the code being tested.
| ### Artifact Structure | ||
|
|
||
| The following directory layout represents an example Energy extension. | ||
| Note the separation of concerns between production code (`src/main`) and testing logic (`src/test`). |
There was a problem hiding this comment.
This is not really the same as what's done in the core repo, where as stated above, there's a dedicated module/project for tests.
|
|
||
| #### Stage 1: Implicit Activation (Current) | ||
|
|
||
| The Manager loads all extensions found via the `ExtensionMetadata` SPI and initializes their associated `AssetModelProvider`, `SetupTasks`, and `ContainerService` implementations automatically. |
There was a problem hiding this comment.
I guess at this stage (without any code change), the current ServiceLoader mechanisms for AssetModelProvider, SetupTasks, and ContainerService would load the implementations irrelevant of the presence of an ExtensionsMetadata or not in the extension jar.
| import org.openremote.model.ExtensionMetadata; | ||
| import java.util.Set; | ||
|
|
||
| public class EnergyExtensionMetadata implements ExtensionMetadata { |
There was a problem hiding this comment.
Is there an added value in having this as a Java class instead of a configuration (json/yaml ...) file ?
Does this allow more build time validation ?
| public ExtensionType getType() { return ExtensionType.COMPOSITE; } | ||
|
|
||
| @Override | ||
| public Set<String> getDependencies() { |
There was a problem hiding this comment.
What's the relationship between this and the dependencies defined via gradle build files ?
|
|
||
| * **Lifecycle Requirement (Restart)**: Because the extension mechanism relies on the Java Classpath and the `ServiceLoader` discovery process, any change to the activation state (enabling or disabling an extension) requires a restart of the Manager container. | ||
| * **User Feedback**: The UI must explicitly notify the user that changes are "Pending" and provide a "Restart Manager" trigger or a clear instruction to restart the stack to apply the new configuration. | ||
| * **Safety Check**: The Manager will validate the pending configuration during the next boot sequence. |
There was a problem hiding this comment.
I guess this is a safety net, but ideally the UI should prevent saving a configuration that would stop the manager from starting after a reboot (that the UI will propose/force on the user).
|
|
||
| ## Versioning & Release Management (Monorepo) | ||
|
|
||
| To minimize administrative overhead and simplify the development lifecycle, all official extensions are managed within a single monorepo. |
There was a problem hiding this comment.
I understand some of the advantages this bring and could see that as a first step but having 2 big monorepos (core + extensions) instead of 1 seems a worse scenario to me.
We'll loose some ease of use for refactoring, debugging, ... and add the complexity of synchronizing the release over 2 repos.
|
|
||
| In this initial phase, explicit core compatibility functions like `getMinCoreVersion()` are omitted from the `ExtensionMetadata` SPI. | ||
|
|
||
| * **The Docker Contract**: Compatibility is managed at the packaging level. Because the extensions and the Manager are bundled into the same Docker image during the CI/CD process, it is guaranteed that the extensions on the classpath have been built and tested against that specific Manager version. |
There was a problem hiding this comment.
Which docker image are we talking about ?
I would think for the "core"/ stock manager image, we don't bundle any extension, the goal being to keep the image lean.
A custom project would build its docker image adding the extension it requires.
We could build a "full" image to ease testing but that shouldn't be the base of all projects. Most projects don't need e.g. code for KNX of Z-Wave.
|
For agents, also ENTSOE, openWeather, battery simulator might become extensions |
|
@remyberden and @pierrekil: Good to think about naming conventions and way we explain from a marketing communication perspective to less/non technical users. |
MartinaeyNL
left a comment
MartinaeyNL
left a comment
There was a problem hiding this comment.
This is a really well written document! Thanks! 👏
Only two small comments from my own pair of eyes. (and field of expertise I guess)
Feel free to respond or explain details.
| energy/ | ||
| ├── src/main/java/org/openremote/extension/energy/ | ||
| │ ├── model/ # Java Asset implementations | ||
| │ │ ├── ElectricityAsset.java | ||
| │ │ └── EnergyModelProvider.java # Implements AssetModelProvider SPI | ||
| │ ├── manager/ # Core logic & Container services | ||
| │ │ └── EnergyOptimisationService.java # Implements ContainerService SPI | ||
| ├── src/main/resources/ | ||
| │ ├── org/openremote/extension/energy/setup/database/ # Flyway scripts | ||
| │ │ └── V20260131_01__RenameEnumValues.sql | ||
| │ └── META-INF/services/ # SPI Registration | ||
| │ ├── org.openremote.model.AssetModelProvider | ||
| │ ├── org.openremote.model.ContainerService | ||
| │ ├── org.openremote.model.ExtensionMetadata | ||
| │ └── org.openremote.model.setup.SetupTasks | ||
| ├── src/test/groovy/org/openremote/extension/energy/ | ||
| │ ├── EnergyOptimisationTest.groovy # Spock Integration Tests | ||
| │ ├── ForecastSolarServiceTest.groovy | ||
| │ └── ManagerTestSetup.groovy # Test-specific environment logic | ||
| └── src/test/resources/ | ||
| └── META-INF/services/ | ||
| └── org.openremote.model.setup.SetupTasks # Test-only setup tasks |
There was a problem hiding this comment.
Out of curiosity, don't extensions have their own build.gradle file?
As in, I assume extensions can have their own third party dependencies outside of OpenRemote.
And the list of "other extensions it depends on" would be in one central place,
so either in build.gradle or inside the org.openremote.model.ExtensionMetadata file, not both.
Or do I misunderstand something?
| The `OR_ENABLED_EXTENSIONS` environment variable is introduced. | ||
|
|
||
| * **Behavior**: The Manager filters the discovered `ExtensionMetadata` list. The Manager automatically activates all transitive dependencies declared in the metadata. | ||
| * **Persistence**: The list of active extension IDs is persisted in `manager_config.json`. |
There was a problem hiding this comment.
I'm totally against persisting this info inside the manager_config.json 😂
It's better to, as a first step, introduce a simple HTTP GET endpoint to retrieve the list of installed extensions.
That would prevent multiple sources of truth, and prevents breaking changes when extending this API.
There was a problem hiding this comment.
Just got context from @wborn on what the persistence is meant for.
Apparently it's the other way around, where we would persist OR_ENABLED_EXTENSIONS inside the manager_config.json file. However, this is not the intended use of this config file, as it's the "configuration of the Manager User Interface". So we'd probably store this in a separate extensions.json file instead.
|
|
||
| In this section, Extensions are categorized by the nature of the resources they provide to the platform. | ||
| There is currently no immediate need for each of these types; however, having a comprehensive list of possible future types helps with thinking about implementation requirements for future extensions. | ||
|
|
There was a problem hiding this comment.
About the extension types, just to note what was discussed, it might be good to stick to tagging extensions with certain categories and not forcing them to be in just "a category".
About the specifics of the types, I would personally stick to our existing slang for most of these, like model, agent, asset type, service, rules, app (custom app), widgets, map, etc. (from a continuity and developer point of view). Of course there is something to say for users that don't know these terms, as you've listed some general terms like utility or UI, etc. which could be helpful for those users. What those should ultimately be I guess we'll figure out while we add more extensions.
I do like you mention more specific "sub" types, like Value Types, Value Filters, Value Formatting, and the rule types.
About specifically Swagger, that could maybe be tagged as an app, and part of a general category like utility.
| Extensions requiring database migrations must use Flyway. | ||
|
|
||
| * **Path**: `src/main/resources/org/openremote/extension/{name}/setup/database/` | ||
| * **Execution**: Migrations are triggered during the extension activation sequence, before `SetupTasks` or `ContainerServices` are initialized. |
There was a problem hiding this comment.
To me database migrations in extensions are scary, especially when they're deleting data. I'm not sure if there are limitations we can put onto them?
5 participants
Introduces the Technical Design Specification for decoupling domain-specific logic (Energy, Protocols, UI) from the platform Core.
Closes openremote/openremote#2327