Skip to content
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

Remove ReloadableServletContainer #24970

Closed
3 tasks
nollymar opened this issue May 17, 2023 · 4 comments · Fixed by #25363
Closed
3 tasks

Remove ReloadableServletContainer #24970

nollymar opened this issue May 17, 2023 · 4 comments · Fixed by #25363

Comments

@nollymar
Copy link
Contributor

Parent Issue

No response

Task

The ReloadableServletContainer must be removed. @spbolton implemented most of the changes on this branch.

What is missing here:

  • Update the branch
  • Remove the reference of the ReloadableServletContainer from the web.xml
  • Testing

Proposed Objective

Core Features

Proposed Priority

Priority 3 - Average

Acceptance Criteria

No response

External Links... Slack Conversations, Support Tickets, Figma Designs, etc.

No response

Assumptions & Initiation Needs

No response

Quality Assurance Notes & Workarounds

No response

Sub-Tasks & Estimates

No response

@dcolina dcolina self-assigned this May 22, 2023
dcolina pushed a commit that referenced this issue May 22, 2023
dcolina pushed a commit that referenced this issue May 23, 2023
dcolina pushed a commit that referenced this issue Jun 27, 2023
…on classes.

* ReloadableServletContainer constructor to call super class initialization.

* DotRestApplication now invokes static method reload on ReloadableServletContainer.

* Reloader class now is static and visible to register into ResourceConfig.
@dcolina dcolina assigned spbolton and unassigned dcolina Jun 29, 2023
nollymar pushed a commit that referenced this issue Jun 30, 2023
* Initial ReloadableServletContainer refactor

* #24970 Refactoring on ReloadableServletContainer and DotRestApplication classes.

* ReloadableServletContainer constructor to call super class initialization.

* DotRestApplication now invokes static method reload on ReloadableServletContainer.

* Reloader class now is static and visible to register into ResourceConfig.

* fixup reloading and cleanup

* fix reload after classes updated

* remove junk

* cleanup

---------

Co-authored-by: spbolton <steve.bolton@dotcms.com>
Co-authored-by: daniel.colina <daniel.colina@dotcms.com>
@nollymar nollymar linked a pull request Jun 30, 2023 that will close this issue
manuelrojas pushed a commit that referenced this issue Jul 5, 2023
* Initial ReloadableServletContainer refactor

* #24970 Refactoring on ReloadableServletContainer and DotRestApplication classes.

* ReloadableServletContainer constructor to call super class initialization.

* DotRestApplication now invokes static method reload on ReloadableServletContainer.

* Reloader class now is static and visible to register into ResourceConfig.

* fixup reloading and cleanup

* fix reload after classes updated

* remove junk

* cleanup

---------

Co-authored-by: spbolton <steve.bolton@dotcms.com>
Co-authored-by: daniel.colina <daniel.colina@dotcms.com>
@dcolina dcolina self-assigned this Jul 10, 2023
@dcolina
Copy link
Contributor

dcolina commented Jul 10, 2023

Needs work.

If you load the REST plugin via web application, it does not appear in the plugin listing, even by forcing a refresh or restarting the framework.

Image

In addition to appearing in the plugin listing, it should be possible to hit the endpoints exposed by the plugin, but these are not available.

See how it should look. This is an example on demo site:

Image

On the other hand, no error is seen in the log traces.

PD:

For testing I'm using com.dotcms.rest

@dcolina dcolina removed their assignment Jul 10, 2023
@spbolton
Copy link
Contributor

@dcolina
Are you running this through the deployWarTomcat with gradle, or through docker container. Does any plugin load, I was testing through the maven build and it was working fine. It maybe the local configuration that the plugin file is not getting dynamically loaded correctly could be configuration of the felix folders which would make sense if you are not seeing any logs as it would not even be attempting to start.

@dcolina
Copy link
Contributor

dcolina commented Jul 10, 2023

Hi Steve, I found that I was having problems with the environment, so the first tests are not valid. I completely restored the local environment, I even had to reboot the laptop to reliably repeat the tests.

Once the local environment was restored, I repeated the above test, adding and removing plugins, and found no errors. You can also see how the added plugins are listed and the endpoints exposed by that plugin can be called.

I followed these steps:

  1. Launch the application
  2. Go to the Plugins section and select Add +.
  3. Select the appropriate plugin and click the Upload button.
  4. When the upload process is finished, the newly added plugin should be listed in the central panel.
  5. You will be able to perform actions such as start, stop, undeploy on the plugin.
  6. Repeat steps i - iv with a REST type plugin to test its functionality.
  7. Once deployed, you will be able to call its exposed REST methods (for the example I tested with the com.dotcms.rest plugin, I called the POST and PUT methods, as you can read in the README file).
  8. No errors are detected, so the internal QA is approved.

Image

Image

Internal QA passed

@josemejias11
Copy link

Approved QA - Tested on master_pre-release_23.08_8d30d6e // Docker // macOS 13.0 // FF v113.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants