Enable ability to provide custom configuration to XMLConfigBuilder

[epochcoder]

Supported via the InputStream/Reader constructors

Default constructors will keep using a new configuration

[harawata]

Thanks for the PR, @epochcoder !

The purpose of XMLConfigBuilder is to build a new Configuration, so accepting an existing instance of Configuration bothers me a little bit.
Also, some settings (e.g. VFS) must be loaded before other settings, but this change makes it impossible to enforce the order.

If XMLConfigBuilder takes Class<? extends Configuration> and instantiates it in the constructor, it still satisfies your need, right?
It would look something like this:

private XMLConfigBuilder(Class<? extends Configuration> configClass, XPathParser parser, String environment, Properties props) {
  super(newConfig(configClass));
  ...
}

private static Configuration newConfig(Class<? extends Configuration> configClass) {
  try {
    return configClass.getConstructor().newInstance();
  } catch (Exception e) {
    throw new BuilderException("Failed to create a new Configuration instance.", e);
  }
}

If the custom Configuration has extra properties, they must be set to the instance returned from the parse() method.

Cc: @brucelwl @zeimao77

[epochcoder]

I agree, it makes sense, basically it guarantees a fresh instance of the config class to be populated, I'll make the proposed changes :)

…

On Thu, Oct 27, 2022, 19:17 Iwao AVE! ***@***.***> wrote: Thanks for the PR, @epochcoder <https://github.com/epochcoder> ! The purpose of XMLConfigBuilder is to build a new Configuration, so accepting an existing instance of Configuration bothers me a little bit. Also, some settings (e.g. VFS) must be loaded before other settings, but this change makes it impossible to enforce the order. If XMLConfigBuilder takes Class<? extends Configuration> and instantiate it in the constructor, it still satisfies your need, right? It would look something like this: private XMLConfigBuilder(Class<? extends Configuration> configClass, XPathParser parser, String environment, Properties props) { super(newConfig(configClass)); ... } private static Configuration newConfig(Class<? extends Configuration> configClass) { try { return configClass.getConstructor().newInstance(); } catch (Exception e) { throw new BuilderException("Failed to create a new Configuration instance.", e); } } If the custom Configuration has extra properties, they must be set to the instance returned from the parse() method. Cc: @brucelwl <https://github.com/brucelwl> @zeimao77 <https://github.com/zeimao77> — Reply to this email directly, view it on GitHub <#2727 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHEYG2JTEUKXF55I6ZBCBLWFK2KVANCNFSM6AAAAAARP57DUA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[coveralls]

Coverage increased (+0.008%) to 87.305% when pulling d7826d7 on epochcoder:feature/2724-pass-subclassed-configuration-xmlconfigbuilder into 043aaaa on mybatis:master.