You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@struts.apache.org by "Stefaan Dutry (JIRA)" <ji...@apache.org> on 2017/05/13 18:55:05 UTC
[jira] [Comment Edited] (WW-4793) Don't add JBossFileManager as a
possible FileManager when not on JBoss
[ https://issues.apache.org/jira/browse/WW-4793?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16009461#comment-16009461 ]
Stefaan Dutry edited comment on WW-4793 at 5/13/17 6:54 PM:
------------------------------------------------------------
[~lukaszlenart]
{quote}
I thought about throwing away that whole FileManager thing as it's only needed in development phase, and instead develop a plugin that can be used during development which will have reloadable implementations of the beans. Another idea is to use commons-io or something to not bother with supporting your own FileManager. My two cents
{quote}
* How do you mean: {{It's only needed in development phase}}
Does that mean it's only used for reloading configurations and the application doesn't need it for initial startup and configuration loading?
* for your other suggestions, if you could point me in the right direction, i wouldn't mind taking a look into them, however i think that's going to be a large modification.
** a plugin for reloadable bean implementations
** using commons-io
For a small modifications i was thinking about one of the following:
* adding a Field Boolean to JBossFileManager so it only checks once:
** code change
{code:java}
...
public class JBossFileManager extends DefaultFileManager {
...
private Boolean supports;
...
@Override
public boolean support() {
if (supports == null) {
supports= isJBoss7() || isJBoss5();
if (supports) {
LOG.debug("JBoss server detected, Struts 2 will use [{}] to support file system operations!", JBossFileManager.class.getSimpleName());
}
}
return supports;
}
...
{code}
** upside: class lookups only happen once
** downside: unboxing of boolean each check
* as sugested, storing the FileManager in the DefaultFileManager
{code}
...
public class DefaultFileManagerFactory implements FileManagerFactory {
...
private FileManager fileManager
...
public FileManager getFileManager() {
if (fileManager == null) {
fileManager = lookupFileManager();
if (fileManager != null) {
LOG.debug("Using FileManager implementation [{}]", fileManager.getClass().getSimpleName());
} else {
fileManager = systemFileManager;
LOG.debug("Using default implementation of FileManager provided under name [system]: {}", systemFileManager.getClass().getSimpleName());
}
fileManager.setReloadingConfigs(reloadingConfigs);
}
return fileManager;
}
...
{code}
I'd like to know your thoughts on this. (and the things i'm missing, especially concerning multithreading, as i'm not an expert there)
was (Author: sdutry):
[~lukaszlenart]
{quote}
I thought about throwing away that whole FileManager thing as it's only needed in development phase, and instead develop a plugin that can be used during development which will have reloadable implementations of the beans. Another idea is to use commons-io or something to not bother with supporting your own FileManager. My two cents
{quote}
* How do you mean: {{It's only needed in development phase}}
Does that mean it's only used for reloading configurations and the application doesn't need it for initial startup and configuration loading?
* for your other suggestions, if you could point me in the right direction, i wouldn't mind taking a look into them, however i think that's going to be a large modification.
** a plugin for reloadable bean implementations
** using commons-io
For a small modifications i was thinking about one of the following:
* adding a Field Boolean to JBossFileManager so it only checks once:
** code change
{code:java}
...
public class JBossFileManager extends DefaultFileManager {
...
private Boolean supports;
...
@Override
public boolean support() {
if (supports == null) {
supports= isJBoss7() || isJBoss5();
if (supports) {
LOG.debug("JBoss server detected, Struts 2 will use [{}] to support file system operations!", JBossFileManager.class.getSimpleName());
}
}
return supports;
}
...
{code}
** upside: class lookups only happen once
** downside: unboxing of boolean each check
* as sugested, storing the FileManager in the DefaultFileManager
{code}
...
public class DefaultFileManagerFactory implements FileManagerFactory {
...
private FileManager fileManager
...
public FileManager getFileManager() {
if (fileManager == null) {
fileManager = lookupFileManager();
}
if (fileManager != null) {
LOG.debug("Using FileManager implementation [{}]", fileManager.getClass().getSimpleName());
} else {
fileManager = systemFileManager;
LOG.debug("Using default implementation of FileManager provided under name [system]: {}", systemFileManager.getClass().getSimpleName());
}
fileManager.setReloadingConfigs(reloadingConfigs);
return fileManager;
}
...
{code}
I'd like to know your thoughts on this. (and the things i'm missing, especially concerning multithreading, as i'm not an expert there)
> Don't add JBossFileManager as a possible FileManager when not on JBoss
> ----------------------------------------------------------------------
>
> Key: WW-4793
> URL: https://issues.apache.org/jira/browse/WW-4793
> Project: Struts 2
> Issue Type: Improvement
> Components: Core
> Affects Versions: 2.5.10
> Reporter: Stefaan Dutry
> Priority: Trivial
> Fix For: 2.5.next
>
>
> When the application starts and there is no FileManager specified as initParam, the {{JBossFileManager}} gets added.
> This results in the check happening everytime a FileManager is requested.
> (When turning on logging, i can see it happening 12 times when starting a simple application)
> {code:java|title=org.apache.struts2.dispatcher.Dispatcher}
> ...
> private void init_FileManager() throws ClassNotFoundException {
> if (initParams.containsKey(StrutsConstants.STRUTS_FILE_MANAGER)) {
> ...
> } else {
> // add any other Struts 2 provided implementations of FileManager
> configurationManager.addContainerProvider(new FileManagerProvider(JBossFileManager.class, "jboss"));
> }
> ...
> }
> ...
> {code}
> {code:java|title=com.opensymphony.xwork2.util.fs.DefaultFileManagerFactory}
> private FileManager lookupFileManager() {
> Set<String> names = container.getInstanceNames(FileManager.class);
> ...
> for (FileManager fm : internals) {
> if (fm.support()) {
> return fm;
> }
> }
> return null;
> }
> {code}
> My suggestion would be to not add it if it's not supported.
> I don't know what the best way to do this would be.
> The possibility i was thinking of so far involves the following:
> * Creating a seperate utility class to check if it can support it
> ** perhaps a public static innerclass of {{JBossFileManager}} with public static method(s) to check it?
> ** or a seperate class (although i think this might be awkward)
> * adding a test around adding the JBossFileManager to only do it when it could be supported.
> additional information:
> * log messages were noticed by adjusting the log4j2 configuration in the {{form-processing}} application from {{struts-examples}} and starting it.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)