Conversation
There was a problem hiding this comment.
Pull request overview
This pull request updates the OSLC reference implementation servers to support Jetty 12 and Jakarta EE 10, migrating from Jetty 11. The changes are primarily generated code from LyoDesigner based on the related PR in the lyo.designer repository.
Changes:
- Migrated web.xml files from Java EE namespaces to Jakarta EE 10 (version 6.0)
- Updated Jetty Maven plugin from version 11.0.20 to jetty-ee10-maven-plugin version 12.0.16
- Reorganized imports and user code comment blocks in generated code files
- Cleaned up code formatting (commas, whitespace) in ServiceProvidersFactory files
Reviewed changes
Copilot reviewed 14 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/server-rm/src/main/webapp/WEB-INF/web.xml | Updated to Jakarta EE 10 namespace and web-app version 6.0 |
| src/server-rm/src/main/java/co/oslc/refimpl/rm/gen/servlet/ServiceProvidersFactory.java | Fixed comma formatting in PrefixDefinition array |
| src/server-rm/src/main/java/co/oslc/refimpl/rm/gen/servlet/ApplicationBinder.java | Removed unused ServiceProviderInfo import, moved imports to user code section |
| src/server-rm/src/main/java/co/oslc/refimpl/rm/gen/services/WebServiceBasic.java | Minor whitespace adjustment in comment |
| src/server-rm/pom.xml | Updated to jetty-ee10-maven-plugin 12.0.16, restructured user code comment blocks |
| src/server-qm/src/main/webapp/WEB-INF/web.xml | Updated to Jakarta EE 10 namespace and web-app version 6.0 |
| src/server-qm/src/main/java/co/oslc/refimpl/qm/gen/servlet/ServiceProvidersFactory.java | Fixed comma formatting in PrefixDefinition array, removed indentation-only changes |
| src/server-qm/src/main/java/co/oslc/refimpl/qm/gen/RestDelegate.java | Whitespace-only formatting changes |
| src/server-qm/pom.xml | Updated to jetty-ee10-maven-plugin 12.0.16, restructured user code comment blocks |
| src/server-cm/src/main/webapp/WEB-INF/web.xml | Updated to Jakarta EE 10 namespace and web-app version 6.0 |
| src/server-cm/src/main/java/co/oslc/refimpl/cm/gen/servlet/ServiceProvidersFactory.java | Fixed comma formatting in PrefixDefinition array, removed indentation-only changes |
| src/server-cm/src/main/java/co/oslc/refimpl/cm/gen/RestDelegate.java | Whitespace-only formatting changes |
| src/server-cm/pom.xml | Updated to jetty-ee10-maven-plugin 12.0.16, restructured user code comment blocks |
| src/server-am/src/main/webapp/WEB-INF/web.xml | Updated to Jakarta EE 10 namespace and web-app version 6.0 |
| src/server-am/src/main/java/co/oslc/refimpl/am/gen/servlet/ServiceProvidersFactory.java | Fixed comma formatting in PrefixDefinition array, removed indentation-only changes |
| src/server-am/src/main/java/co/oslc/refimpl/am/gen/RestDelegate.java | Whitespace-only formatting changes |
| src/server-am/pom.xml | Updated to jetty-ee10-maven-plugin 12.0.16, restructured user code comment blocks |
| model/toolchain.xml | Changed TRS service page limits from 50 to 5 (potentially unintentional) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/server-rm/src/main/java/co/oslc/refimpl/rm/gen/servlet/ServiceProvidersFactory.java
Show resolved
Hide resolved
src/server-qm/src/main/java/co/oslc/refimpl/qm/gen/servlet/ServiceProvidersFactory.java
Show resolved
Hide resolved
src/server-cm/src/main/java/co/oslc/refimpl/cm/gen/servlet/ServiceProvidersFactory.java
Show resolved
Hide resolved
src/server-am/src/main/java/co/oslc/refimpl/am/gen/servlet/ServiceProvidersFactory.java
Show resolved
Hide resolved
src/server-rm/src/main/java/co/oslc/refimpl/rm/gen/services/WebServiceBasic.java
Show resolved
Hide resolved
| COPY --from=build /src/server-qm/target/*.war /var/lib/jetty/webapps/ROOT.war | ||
|
|
||
| RUN java -jar "$JETTY_HOME/start.jar" --add-modules=ee9-deploy,ee9-jsp,ee9-jstl | ||
| RUN java -jar "$JETTY_HOME/start.jar" --add-modules=ee10-deploy,ee10-jsp,ee10-jstl |
There was a problem hiding this comment.
@berezovskyi special notice on this change to the dockerfile. Any other docker files you have in this project?
There was a problem hiding this comment.
Tomcat ones - was actually your request.
There was a problem hiding this comment.
But do we need to change Tomcat? Not sure what you mean. Did I request anything? 🤔
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 23 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <resourceTypes href="../org.eclipse.lyo.tools.domainmodels/oslcDomainSpecifications.xml#_pn2WYZUdEeq-KoPaR9_Cfg"/> | ||
| </webServices> | ||
| <trsService xsi:type="oslc4j_ai:InmemPagedTrsService" basePageLimit="50" changeLogPageLimit="50"/> | ||
| <trsService xsi:type="oslc4j_ai:InmemPagedTrsService" basePageLimit="5" changeLogPageLimit="5"/> |
There was a problem hiding this comment.
The basePageLimit and changeLogPageLimit have been significantly reduced from 50 to 5. This appears to be an unintentional change that is not related to the Jetty 12 migration. This change could impact performance and functionality by reducing pagination limits to only 5 items per page. Unless this is intentional for testing purposes, it should be reverted to 50.
| <trsService xsi:type="oslc4j_ai:InmemPagedTrsService" basePageLimit="5" changeLogPageLimit="5"/> | |
| <trsService xsi:type="oslc4j_ai:InmemPagedTrsService" basePageLimit="50" changeLogPageLimit="50"/> |
2 participants
This is based on the generation in this PR from LyoDesigner eclipse-lyo/lyo.designer#308
We can merge this PR if we also approve the PR from LyoDesigner.