Update the module to Platform 3.x #221
Conversation
947106b to
93606dd
Compare
| </property> | ||
| </bean> | ||
|
|
||
| <bean parent="obsServiceTarget" > |
There was a problem hiding this comment.
Do you mind explaining why we removed this?
There was a problem hiding this comment.
Having this causes an error.
Caused by: org.springframework.beans.factory.NoSuchBeanDefinitionException: No bean named 'obsServiceTarget' available
| <!-- Add here beans related to the web context --> | ||
|
|
||
| <bean id="legacyUiUrlMapping" class="org.springframework.web.servlet.handler.SimpleUrlHandlerMapping"> | ||
| <property name="patternParser"><null/></property> |
There was a problem hiding this comment.
Why add it if the value is null?
There was a problem hiding this comment.
Caused by: org.springframework.web.util.pattern.PatternParseException: No more pattern data allowed after {*...} or ** pattern element
This is a hack I used to avoid PatternParseException.
So what's happening is that there are two "org.springframework.web.servlet.mvc.SimpleControllerHandlerAdapter" beans registered in the context. One we explicitly register here and one that is implicitly registered here by the use of the The bean we explicitly create gets named I'm not entirely sure what happened, but this commit is part of Spring 6 and it will just remove the implicit alias of So a couple of things suggest themselves:
Personally I think 3 is the cleanest thing, but may require more tweaks, e.g., to ensure that the new |
@ibacher I tried both 1 and 3. However, I am still seeing the error. Here are the PRs for them. |
|
Well, I stupidly assumed that the XML stuff wouldn't interfere with the |
|
Anyways, @wikumChamith I added a commit here and to openmrs/openmrs-core#5374 that fixes that issue in a way I'm happy with. There are still some test failures but they aren't caused by this issue. Btw, we should take every opportunity available to dump XML Spring configurations if possible. There were some issues with the services wrapped in |
|
PR to fix test errors on ChangePasswordFormControllerTest: openmrs/openmrs-core#5394 |
|
I put a comment on that pull request. |
|
@wikumChamith did you eventually figure this out? |
|
@wikumChamith in that failing test, try changing |
|
@dkayiwa, @ibacher the module is compiling now, but I’m seeing an error when I run the module: To resolve this, I removed the taglib dependencies from core and used org.glassfish.web:jakarta.servlet.jsp.jstl in this module. However, I’m still seeing the same error. We’re currently using the JstlCoreTLV validator, but since it now uses Jakarta EE, removing it didn’t fix the issue either. Is there anywhere else that might still be using a javax-era taglib that I may have missed? |
|
@wikumChamith did you figure this out? |
Nope, still trying to sort it out. Any suggestions?? |
| Concept concept = cs.getConcept(5497); | ||
| //sanity check, the current preferred Name should be different from what will get set in the form | ||
| Assert.assertNotSame("CD3+CD4+ABS CNT", concept.getPreferredName(britishEn).getName()); | ||
There was a problem hiding this comment.
Is there a reason why you did not use a different pull request for these formatting changes?
| ProgramFormController controller = (ProgramFormController) applicationContext.getBean("programForm"); | ||
| controller.handleRequest(request, new MockHttpServletResponse()); | ||
|
|
||
| Context.clearSession(); |
There was a problem hiding this comment.
What failure is the above addressing?
|
@wikumChamith because |
@dkayiwa, that also gives a similar error: I also tried deleting all the |
|
Even after removing the validator? |
Yes |
|
Can you open a pull request for the changes you did in core for this? |
|
|
How are you running it? I seem to be getting a different error: |
I tried running it with both the SDK and |
|
Yeah, I tried with Claude, but it just kept suggesting random changes :) I decided to set Spring’s log level to DEBUG and noticed this: The forwarding path has a double slash (/WEB-INF/view//scripts/), which is likely causing the issue. What I’m wondering now is why we have the jspViewResolver and urlMapping beans declared twice, once in WebConfig (moved from openmrs-servlet.xml) and again in openmrs_static_content-servlet.xml. |
|
@wikumChamith aren't you the one who duplicated them? 😊 |
No, the duplication was already there. I only moved the beans from openmrs-servlet.xml to WebConfig. Here’s the proof from 2.8.x: |
|
Do they deal with the same suffix? |
|
No, that’s why I’m wondering why both beans have the same name :) |
|
Change the name and see what happens. 😊 |
Nothing changes and I’m still getting the same error. From the logs, you can see .jsp is being appended to .js requests:
So it looks like these requests are going through the jspViewResolver in WebConfig :) |
…and updating dependencies
… for Jakarta EE compatibility.
…d centralize fallback logic.
|
@dkayiwa what is the reason for the force push :) ? |
|
@wikumChamith i just clicked the update branch button 😊 |
This would be the case, but only within the scope of the Not sure what to suggest here. We could try:
|
No description provided.