struts-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Stefaan Dutry (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (WW-4793) Don't add JBossFileManager as a possible FileManager when not on JBoss
Date Mon, 08 May 2017 20:36:04 GMT

    [ https://issues.apache.org/jira/browse/WW-4793?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16001477#comment-16001477
] 

Stefaan Dutry commented on WW-4793:
-----------------------------------

[~lukaszlenart]

{quote}
If you check usage of the FileManagerFactory#getFileManager() method you will notice it's
used 6 times (7 to be correct but AnnotationActionValidatorManager is used by default instead
of DefaultActionValidatorManager), so 12 calls is ok, 6 times when booting, and 6 times when
reloading the framework after the initial boot.
{quote}
It's true that this is only done at the moment of booting or reloading the framework, and
therefore will never cause a performance issue. My thought was that this was a redundant thing
to keep checking when you know it's not going to change after the first check, and in that
way, even adding it to be checked every time. (technicaly support might be added at runtime
using reflection on the classloader)

{quote}
And that static method does exactly what the FileManager's support method does but right now
you have introduced a hard dependency between Dispatcher and JBossFileManager using the static
method - I'm not a fan of such things.
{quote}
I'm afraid i don't fully understand what you mean here. How does adding a static method to
the class that's already been referenced, and as far as i can tell, would always get instantiated,
make this dependency harder? Please elaborate so i can take this into account in the future.

{quote}
The best option I see here is to remove the line configurationManager.addContainerProvider(new
FileManagerProvider(JBossFileManager.class, "jboss")); and put a note into docs about how
to configure Struts 2 when running on JBoss.
{quote}
Does this mean that the code wouldn't run on JBoss if the JBossFileManager isn't present in
the configuration? If that's the case, i'd rather keep it as it is than remove the class.
(maybe adding a minor configuration optimization to the docs for when not running on JBoss
then)

Thx for the feedback.

> 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)

Mime
View raw message