[JSPWIKI-1172] Set jspwiki.workDir default to servlet context temp directory #282
Conversation
arturobernalg
force-pushed
the
feature/JSPWIKI-1172
branch
from
d4b7db5
to
ed23a5a
Compare
| * | ||
| * @since 2.12.0 | ||
| */ | ||
| public class WikiServletContextListener implements ServletContextListener { |
There was a problem hiding this comment.
Hi @arturobernalg , thanks for looking into this!
Instead of adding a new servlet, I think it would be better to go through WikiBootstrapServletContextListener, which calls Wiki.init( servletContext), which in turn delegates to PropertyReader.loadWebAppProps( servletContext ) to load the wiki properties (from jspwiki[-custom].properties, system env & props, etc.). I'd check there for jspwiki.workDir existence. If it's set, then we do nothing. If it's missing, then we set it to javax.servlet.context.tempdir (or, if this doesn't exist, java.io.tmpdir) on the properties object.
Once the jspwiki.workDir is set on PropertyReader, then we can remove the java.io.tmpdir usage on WikiEngine, and refactor Installer to use the wiki properties instead of directly using java.io.tmpdir.
WDYT?
There was a problem hiding this comment.
I've made the changes as per your suggestion. I modified WikiBootstrapServletContextListener to check and set 'jspwiki.workDir' and refactored Installer to use this property instead of 'java.io.tmpdir'. I also removed the direct usage of 'java.io.tmpdir' in WikiEngine. Please have a look at the latest commit.
Thanks for your valuable feedback!
arturobernalg
force-pushed
the
feature/JSPWIKI-1172
branch
from
bddd321
to
90b32bf
Compare
| * @param servletContext the Servlet context from which to fetch the tempdir if needed | ||
| * @since JSPWiki 2.11.1 | ||
| */ | ||
| void setWorkDir(final Properties properties, final ServletContext servletContext) { |
There was a problem hiding this comment.
Hi,
my bad, I was meaning do all of these changes inside PropertyReader, just before the variable expansion. That way all property reading/mungling is done inside PropertyReader#loadWebAppProps
| @@ -86,6 +86,7 @@ public Installer( final HttpServletRequest request, final ServletConfig config ) | |||
| // Stash the request | |||
| m_request = request; | |||
| m_validated = false; | |||
| TMP_DIR = m_engine.getWikiProperties().getProperty( "java.io.tmpdir" ); | |||
There was a problem hiding this comment.
should be m_engine.getWikiProperties().getProperty( "jspwiki.workDir" )
This commit makes a few key changes: 1. PropertyReader now checks for the existence of jspwiki.workDir. If it's missing, the servlet context's temporary directory (or the system's temporary directory if that's unavailable) is set as the working directory. 2. Removed direct usage of java.io.tmpdir in WikiEngine since jspwiki.workDir is now ensured to be set in PropertyReader. 3. Refactored the Installer to use wiki properties (specifically, jspwiki.workDir) instead of directly using java.io.tmpdir. These changes streamline the handling of the working directory and ensure that it's consistently sourced from the same place (the wiki properties).
arturobernalg
force-pushed
the
feature/JSPWIKI-1172
branch
from
84299ef
to
28feea1
Compare
|
merged in 2.12.1-git-02, would you mind resolving the associated jira issue? thx! |
Description:
This pull request addresses the issue JSPWIKI-1172 by changing the default value of jspwiki.workDir from java.io.tmpdir to the servlet container's context-specific temp directory, as provided by the "javax.servlet.context.tempdir" context attribute.
Key Changes:
Testing: