jsonschema2pojo icon indicating copy to clipboard operation
jsonschema2pojo copied to clipboard

Add configuration map (customRuleFactoryConfiguration) for Custom Rule Factories

Open ddcruver opened this issue 5 years ago • 4 comments
trafficstars

This will allow custom rule factories to have a simple string to string map of configuration properties. For example in maven this allows a property such as this to be defined:

<plugin>
	<groupId>org.jsonschema2pojo</groupId>
	<artifactId>jsonschema2pojo-maven-plugin</artifactId>
	<version>1.0.3-SNAPSHOT</version>
	<executions>
		<execution>
			<goals>
				<goal>generate</goal>
			</goals>
		</execution>
	</executions>
	<dependencies>
		<dependency>
			<groupId>org.example.jsonschema2pojo-extensions</groupId>
			<artifactId>org.example.jsonschema2pojo-extensions.custom-rule-factory</artifactId>
			<version>${project.version}</version>
		</dependency>
	</dependencies>

	<configuration>
		<customRuleFactory>org.example.extensions.rules.CustomRuleFactory</customRuleFactory>

		<customRuleFactoryConfiguration>
			<quantitiesAsString>true</quantitiesAsString>
		</customRuleFactoryConfiguration>
	</configuration>
</plugin>

This allows the custom rule factory to have configuration in the configuration section of the plugin without fear of it being an invalid configuration.

ddcruver avatar Apr 08 '20 11:04 ddcruver

For a little context without something like this I would have to do this to get a configuration value from a Maven POM:

        protected Boolean getConfigurationValueAsBooleanParseMavenConfiguration(String configurationField, boolean defaultValue)
	{
		GenerationConfig generationConfig = this.getGenerationConfig();

		if (generationConfig instanceof AbstractMojo)
		{
			AbstractMojo abstractMojo = (AbstractMojo) generationConfig;

			Object pluginDescriptor = abstractMojo.getPluginContext().get("pluginDescriptor");

			if (pluginDescriptor instanceof PluginDescriptor)
			{
				Object configuration = ((PluginDescriptor) pluginDescriptor).getPlugin().getConfiguration();

				if (configuration instanceof Xpp3Dom)
				{
					Xpp3Dom xpp3Dom = (Xpp3Dom) configuration;
					Xpp3Dom configNode = xpp3Dom.getChild(configurationField);

					if (configNode != null && StringUtils.isNotBlank(configNode.getValue()))
					{
						getLogger().debug("Config Value for " + configurationField + ": " + configNode.getValue());
						return configNode.getValue().equalsIgnoreCase("true");
					}
				} else
				{
					getLogger().error(CustomRuleFactory.class.getCanonicalName() +
											  " found unexpected configuration object of type " + configuration.getClass().getCanonicalName() +
											  " expected " + Xpp3Dom.class.getCanonicalName() +
											  "; will not be able to resolve " + configurationField);
				}
			} else
			{
				getLogger().error(CustomRuleFactory.class.getCanonicalName() + " could not get Plugin Descriptor from Plugin Context; will not be able to resolve " + configurationField);
			}
		} else
		{
			getLogger().warn(CustomRuleFactory.class.getCanonicalName() + " only supports Maven execution; will not be able to resolve; will not be able to resolve  " + configurationField);
		}

		return defaultValue;
	}

This limits the custom rule factory to maven and is dependent on internals of Maven that may change between releases and will be marked as invalid XML in IDE that validate Maven POMs.

The commit solution will eliminate the need for any of this and it can be accessed like this (in RuleFactory):

        protected Boolean getConfigurationValueAsBooleanUsingNewApi(String configurationField, boolean defaultValue)
	{
		Map<String, String> customRuleFactoryConfiguration = getGenerationConfig().getCustomRuleFactoryConfiguration();
		return getBooleanValue(configurationField, customRuleFactoryConfiguration, defaultValue);
	}

	private Boolean getBooleanValue(String configurationField, Map<String, String> configMap, boolean defaultValue)
	{
		if (configMap != null)
		{
			String fieldValue = configMap.get(configurationField);

			if (StringUtils.isNotBlank(fieldValue))
			{
				return fieldValue.equalsIgnoreCase("true");
			}
		}

		return defaultValue;
	}

ddcruver avatar Apr 08 '20 11:04 ddcruver

I'm a fan, and think it significantly reduces the barrier to entry for writing your own custom rule factory.

dewthefifth avatar Apr 08 '20 13:04 dewthefifth

@joelittlejohn Could we get this pull request in? Any concerns or objections?

ddcruver avatar Jun 04 '20 21:06 ddcruver

I like this, and I think we should definitely take many parts of it, however I wonder about the implementation here. I think we might be able to do it in a less intrusive way, without introducing a new Abstract class which all rules must extend.

How about this: When we instantiate a custom rule factory class, if we find a constructor that takes a GenerationConfig, we can use it. If there is no constructor that takes a GenerationConfig, we can use the default constructor as we do now. The rule factory class can easily pass this GenerationConfig value on to any rules it creates.

All existing rule factories can easily support this feature by just adding a new constructor.

Another, even more 'future proof' and extensible version of this would be: when we instantiate a new custom rule factory, we look for a method like setGenerationConfig(GenerationConfig). If this method exists, we set the generation config. This would allow more fields like this to be added in future.

joelittlejohn avatar Mar 22 '21 18:03 joelittlejohn