Hi Mathieu, This is code which I refactored from a very messy startup code, and yes you are right, we don't need java reflection, and we do not even need a startup loader in the first place. The code could be much simpler.
We used to have 2 startup loaders, the existing one, and also a loader for the GUI POS system which we deleted during refactoring. The idea back in the day was that the startup code could be instantiated by multiple loaders to customize the behavior of the system. The refactoring work is not yet done, and we got side tracked by many issues, but I would say maybe 50% or more of the existing java code base could be trimmed down and simplified. My recommendation is to completely delete the startup loaders API, but there are a few technical challenges to it because there is some state linked to it here and there, so it's a matter of catching everything, re-routing ,and getting a simpler startup logic. On Mon, Oct 22, 2018 at 12:37 AM Mathieu Lirzin <[email protected]> wrote: > > Hello, > > I have a question regarding the > ‘org.apache.ofbiz.base.start.StartupControlPanel#loadStartupLoaders()’ > current implementation: > > --8<---------------cut here---------------start------------->8--- > private static void loadStartupLoaders(Config config, > List<StartupLoader> loaders, > List<StartupCommand> ofbizCommands, > AtomicReference<ServerState> serverState) throws StartupException > { > > String startupLoaderName = > "org.apache.ofbiz.base.container.ContainerLoader"; > ClassLoader classloader = > Thread.currentThread().getContextClassLoader(); > > synchronized (loaders) { > if (serverState.get() == ServerState.STOPPING) { > return; > } > try { > Class<?> loaderClass = > classloader.loadClass(startupLoaderName); > StartupLoader loader = (StartupLoader) > loaderClass.newInstance(); > loaders.add(loader); // add before loading, so unload can > occur if error during loading > loader.load(config, ofbizCommands); > } catch (ReflectiveOperationException e) { > throw new StartupException(e); > } > } > serverState.compareAndSet(ServerState.STARTING, ServerState.RUNNING); > } > --8<---------------cut here---------------end--------------->8--- > > I don't understand the goal of using reflection for instantiating the > ‘ContainerLoader’ class. Can't we just have something like the following > instead? > > --8<---------------cut here---------------start------------->8--- > private static void loadStartupLoaders(Config config, > List<StartupLoader> loaders, > List<StartupCommand> ofbizCommands, > AtomicReference<ServerState> serverState) throws StartupException > { > > ClassLoader classloader = > Thread.currentThread().getContextClassLoader(); > > synchronized (loaders) { > if (serverState.get() == ServerState.STOPPING) { > return; > } > StartupLoader loader = new ContainerLoader(); > loaders.add(loader); // add before loading, so unload can occur > if error during loading > loader.load(config, ofbizCommands); > } > serverState.compareAndSet(ServerState.STARTING, ServerState.RUNNING); > } > --8<---------------cut here---------------end--------------->8--- > > If this is required, I will be happy if someone could add an inline > comment giving a rationale for the current implementation. If that's > not the case, I can open a JIRA with a proper patch if needed. > > Thanks. > > -- > Mathieu Lirzin > GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
