-
Notifications
You must be signed in to change notification settings - Fork 18
feat: Extract env/system/context config logic into LyoConfigUtil #220
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Discussed with Jad. Andrew will
|
4d6ec16 to
efbc772
Compare
|
SonarCloud Quality Gate failed. |
|
Fails due to #284, we should instead move this code to a new module, e.g. |
b4956bb to
25e6482
Compare
70ed956 to
1aab254
Compare
3e802c4 to
8454ba0
Compare
8454ba0 to
1c662e7
Compare
|
eclipse-lyo/lyo.designer#295 was just merged and involves opening up the static method that this PR refactors. Would be good to merge it before 7.0 |
Signed-off-by: Andrew Berezovskyi <andriib@kth.se>
| * @param klass Class of the ServletListener | ||
| * @return value, if found, from ENV, JVM, or Servlet Context (in this order) | ||
| */ | ||
| public static String getOslcConfigProperty( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reflecting on this, I have two remarks:
The time of hosting 10s of adaptors in a single large JEE server is over, we can expect no clashes with other Lyo applications (not sure about RCP-hosted OSGi extensions using multiple Lyo servers - @jhemm2 are you using Lyo in this manner?). We still need to distinguish Lyo props from any other props user code might rely. Instead of co.oslc.refimpl.am.gen.servlet.cors.friends, we can do org.eclipse.lyo.cors.friend.
We should also align ourselves on https://microprofile.io/specifications/microprofile-config-2/ if possible. Ideally, we want to adopt it wholesale. Otherwise, we want to follow their rules, at least. This may involve a bit of a breaking change - they suggest to map the org.eclipse.lyo.cors.friend (btw, I think we should call it friends instead of friend) prop to the ORG_ECLIPSE_LYO_CORS_FRIEND unlike our LYO_CORS_FRIEND.
I am not against making a breaking change in 7.0, but we should have a definitive audit of all existing props in use and a migration path. @Jad-el-khoury if you think the breaking changes are not worth it, I am also OK to keep prop naming as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My preliminary analysis points at the following servlet props being in use in web.xml files:
- baseurl
- host
- port
- scheme
- store.query
- store.update
And these ones I suspect to be non-standard:
- prorDataDir
- repository.dir
- root.repository.dir
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces the LyoConfigUtil class to centralize configuration property lookup across environment variables, JVM properties, and servlet context parameters, while also performing cleanup tasks including Log4j remnant removal and deprecating legacy client code.
- Extract configuration logic into
LyoConfigUtilwith prioritized property lookup - Add new
lyo-server-commonmodule withLyoAppConfigurationrecord - Clean up logging configuration and deprecate
OslcRestClient
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| core/oslc4j-utils/src/main/java/org/eclipse/lyo/core/utils/marshallers/LyoConfigUtil.java | New utility class providing centralized configuration property lookup with environment/JVM/context priority |
| server/lyo-server-common/src/main/java/org/eclipse/lyo/server/common/LyoAppConfiguration.java | New configuration record for Lyo server applications |
| server/lyo-server-common/pom.xml | New Maven module for common server utilities |
| Various simplelogger.properties files | Updated logging configurations with reasonable defaults for dependencies |
| pom.xml files | Module structure updates and dependency management changes |
| @@ -0,0 +1,116 @@ | |||
| package org.eclipse.lyo.core.utils.marshallers; | |||
Copilot
AI
Aug 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The class lacks a copyright header and class-level documentation. Add the standard Eclipse copyright header and document the class purpose, usage patterns, and examples.
core/oslc4j-utils/src/main/java/org/eclipse/lyo/core/utils/marshallers/LyoConfigUtil.java
Outdated
Show resolved
Hide resolved
core/oslc4j-utils/src/main/java/org/eclipse/lyo/core/utils/marshallers/LyoConfigUtil.java
Outdated
Show resolved
Hide resolved
core/oslc4j-utils/src/main/java/org/eclipse/lyo/core/utils/marshallers/LyoConfigUtil.java
Outdated
Show resolved
Hide resolved
server/oslc4j-registry/src/main/resources/simplelogger.properties
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@oslc-bot /test-all |
|
@oslc-bot /test-all |








Description
As you know, https://blog.sonatype.com/a-new-0-day-log4j-vulnerability-discovered-in-the-wild is a big problem for the ecosystem. We are not using v2 of Log4j and are not affected but I wanted to take a bit of time to fully remove remnants of Log4j use in Lyo. As you know, Lyo uses SLF4J API in all SDK libraries and SLF4J + SimpleLogger in the webapps we ship.
Another thing this PR does is introduce the
org.eclipse.lyo.core.utils.marshallers.LyoConfigUtilclass with helper methods to scan the environment for a property. For example, if you are looking for theportproperty in theorg.eclipse.lyo.oslc4j.core.servlet.ServletListenerinstance, we want to check these places in the order of priority:LYO_PORTenvironment variable. This is the new override mechanism for containerized environments.org.eclipse.lyo.oslc4j.core.servletSystem (JVM) property. This is the old override mechanism for multitenant application server environments.org.eclipse.lyo.oslc4j.core.servletServlet Context parameter. This is the default configuration mechanism for Lyo.org.eclipse.lyo.core.utils.marshallers.LyoConfigUtil#getOslcConfigPropertydoes exactly that, whileorg.eclipse.lyo.core.utils.marshallers.LyoConfigUtil#getOslcConfigPropertyNoContextrepresents a fallback mechanism only looking in (1) and (2) in case it must be invoked from a place where aServletContextreference is not available.However, after getting the
oslc4j-registryapp to run, I realized it only contains a CF only on aServiceProviderlevel. My old impression was that it allows to run a single registry where mutliple SP Catalogs can be registered with the "registry" SP Catalog. In light of this discovery, I propose to markoslc4j-registryfor removal in Lyo 5.0. In current form, it's not much more than a demo app and a default output from Lyo Designer has more user-friendly look and features out of the box.Finally,
oslc4j-winkseems to have only 2 useful things:OslcResourceShapeResourcefor reuse by Wink-based OSLC Servers. We don't support Wink any more and Lyo Designer supports much more feature-rich shape resource support (e.g. Shape HTML tables like in the OSLC specs, even for custom resources).org.eclipse.lyo.oslc4j.client.OslcRestClient. This client was long replaced by the (now obsolete)org.eclipse.lyo.client.oslc.OslcClient. Modern apps should use one of the new clients via theorg.eclipse.lyo.client.IOslcClientinterface.With this in mind, I propose to mark the
org.eclipse.lyo.oslc4j.client.OslcRestClientdeprecated immediately (done in this PR) and also scheduleoslc4j-winkfor full removal from Lyo in 5.0.Checklist
<exclude>log4j:log4j</exclude>