[
https://issues.apache.org/jira/browse/WW-5075?focusedWorklogId=461860&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-461860
]
ASF GitHub Bot logged work on WW-5075:
--------------------------------------
Author: ASF GitHub Bot
Created on: 22/Jul/20 05:43
Start Date: 22/Jul/20 05:43
Worklog Time Spent: 10m
Work Description: lukaszlenart commented on a change in pull request #427:
URL: https://github.com/apache/struts/pull/427#discussion_r458546820
##########
File path:
plugins/osgi/src/main/java/org/apache/struts2/osgi/BundlePackageLoader.java
##########
@@ -45,24 +47,96 @@
private static final Logger LOG =
LogManager.getLogger(BundlePackageLoader.class);
+ private Container contextContainer = null;
+
+ @Override
public List<PackageConfig> loadPackages(Bundle bundle, BundleContext
bundleContext, ObjectFactory objectFactory,
Review comment:
Maybe mark this `@Deprecated`?
##########
File path:
plugins/osgi/src/main/java/org/apache/struts2/osgi/BundlePackageLoader.java
##########
@@ -45,24 +47,96 @@
private static final Logger LOG =
LogManager.getLogger(BundlePackageLoader.class);
+ private Container contextContainer = null;
+
+ @Override
public List<PackageConfig> loadPackages(Bundle bundle, BundleContext
bundleContext, ObjectFactory objectFactory,
FileManagerFactory
fileManagerFactory, Map<String, PackageConfig> pkgConfigs) throws
ConfigurationException {
+ if (pkgConfigs == null) {
+ throw new IllegalArgumentException("Cannot load packages from a
null package configuration"); // Better than a NPE.
+ }
+
Configuration config = new DefaultConfiguration("struts.xml");
+
+ LOG.trace("LoadPackages - After config constructed. Before
BundleConfigurationProvider constructed");
+
BundleConfigurationProvider prov = new
BundleConfigurationProvider("struts.xml", bundle, bundleContext);
+
+ LOG.trace("LoadPackages - After BundleConfigurationProvider
constructed. Before config.addPackageConfig loop");
+
for (PackageConfig pkg : pkgConfigs.values()) {
config.addPackageConfig(pkg.getName(), pkg);
}
+
+ LOG.trace("LoadPackages - After config.addPackageConfig loop. Before
prov.setObjectFactory()");
+
prov.setObjectFactory(objectFactory);
+
+ final DefaultFileManagerFactory defaultFileManagerFactory = new
DefaultFileManagerFactory();
+ final Container container = getContextContainer();
+
+ if (container == null) {
+ LOG.warn("LoadPackages - Config Container is null. May cause a
NPE to be thrown");
+ }
+
+ defaultFileManagerFactory.setContainer(container);
+
+ if (fileManagerFactory == null || fileManagerFactory.getFileManager()
== null) {
+ LOG.warn("LoadPackages - FileManagerFactory parameter is null or
produces a null FileManager, replacing with a new DefaultFileManagerFactory
instance");
+ fileManagerFactory = defaultFileManagerFactory;
+ }
+
prov.setFileManagerFactory(fileManagerFactory);
Review comment:
I would change this a bit because right now you are re-assigning to the
method's parameter, also you are creating `DefaultFileManagerFactory` upfront
even if it could be not needed. What about such change:
```java
if (fileManagerFactory == null || fileManagerFactory.getFileManager() ==
null) {
LOG.warn("LoadPackages - FileManagerFactory parameter is null or produces
a null FileManager, replacing with a new DefaultFileManagerFactory instance");
final DefaultFileManagerFactory defaultFileManagerFactory = new
DefaultFileManagerFactory();
final Container container = getContextContainer();
if (container == null) {
LOG.warn("LoadPackages - Config Container is null. May cause a NPE to
be thrown");
} else {
container.inject(defaultFileManagerFactory);
}
prov.setFileManagerFactory(defaultFileManagerFactory);
} else {
prov.setFileManagerFactory(fileManagerFactory);
}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 461860)
Time Spent: 40m (was: 0.5h)
> Upgrade OSGi to the latest version
> ----------------------------------
>
> Key: WW-5075
> URL: https://issues.apache.org/jira/browse/WW-5075
> Project: Struts 2
> Issue Type: Dependency
> Components: Plugin - OSGi
> Reporter: Lukasz Lenart
> Priority: Major
> Fix For: 2.5.24, 2.6
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> Currently the OSGi plugin is using
> {code:xml}
> <dependency>
> <groupId>org.osgi</groupId>
> <artifactId>org.osgi.core</artifactId>
> <version>4.3.1</version>
> </dependency>
> {code}
> but there is a new version 6.0.0
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
