Re: Pluggable template retrival/caching logic (Was: Template Loader Implementation that loads from a database (DataSource))
This is cleaner, more obvious what's going on underneath, but since the DefaultTemplateResolver will be the most commonly used you could just leave the current setting methods as they are and just document that if you set a different TemplateResolver they will be ignored. -David On Wed, 2017-02-15 at 14:56 +0100, Daniel Dekany wrote: > An interesting consequence of introducing the TemplateResolver concept > is that it the earlier top-level configuration settings, like > templateLoader, templateLookupStrategy, templateNameFormat, > cacheStorage, and templateUpdateDelayMillisecond should now be the > settings (JavaBean properties) of DefaultTemplateResolver, not of > Configuration. In theory at least. It's the same things as with > ObjectWrapper, where multiple implementations are possible, and each > has different settings. > > This has some inconvenient consequences for the majority who uses the > DefaultTemplateResolver. Earlier, you have written: > > cfg.setTemplateLoader(new FooTemplateLoader()) > cfg.setTemplateUpdateDelayMilliseconds(10_000); > > but now you had to write either: > > DefaultTemplateResolver dtr = new DefaultTemplateResolver(); > dtr.setTemplateLoader(new FooTemplateLoader()); > dtr.setTemplateUpdateDelayMilliseconds(10_000); > cfg.setTemplateResolver(dtr); > > or alternatively: > > DefaultTemplateResolver dtr = (DefaultTemplateResolver) > dtr.getTemplateResolver(); > dtr.setTemplateLoader(new FooTemplateLoader()); > dtr.setTemplateUpdateDelayMilliseconds(10_000); > > Also configuring with j.u.Properties becomes different, because you > can't have this anymore: > > templateLoader = com.example.FooTemplateLoader > templateUpdateDelay = 10s > > I guess instead it should be something like: > > templateResolver.templateLoader = com.example.FooTemplateLoader > templateResolver.templateUpdateDelay = 10s > > which is a syntax that we don't yet support (dotted setting names). > This syntax we do support though (in FM2): > > templateResolver=DefaultTemplateResolver( \ > templateLoader = com.example.FooTemplateLoader, \ > templateUpdateDelayMilliseconds = 10_000 \ > ) > > but it has the problem that you can't, for example, set the > templateUpdateDelay without also specifying the whole templateResolver > with all of its settings. So that's where the dotted notation is > better. OTOH for some other things the above syntax is better, like > for specifying the templateConfigurations (see > http://freemarker.org/docs/pgui_config_templateconfigurations.html). > So I think we should have both, and since we must ensure that > templateResolver=com.example.MyTemplateResolver is "executed" earlier > than templateResolver.mySetting=foo anyway, if mySetting is specified > on both places (i.e., you have > templateResolver=com.example.MyTemplateResolver(mySetting="foo") > there), then obviously templateResolver.mySetting wins. > > And if we support the dotted notation in j.u.Properties, that should > work for the objectWrapper setting too, naturally. > > >
Re: [FM3] Configuration mutability (Was: [FM3] Configuration API simplification)
On Wed, Feb 15, 2017 at 9:43 AM, Daniel Dekany wrote: > BTW, Configuration.Builder or ConfigurationBuilder? Both convention > exists in the wild. I have recently noticed that the Java API is using > Xxx.Builder at some places, so maybe that will be the more popular > convention. It surely has the advantage that it can call a private > constructor of the built class (or access other private members of > it). I'm fine with either way, but Configuration.Builder seems to have more benefit like you mentioned, trading off the code size in single file (which might be okay with help of IDEs nowadays). Woonsan > > Wednesday, February 15, 2017, 12:30:30 PM, brede...@me.com wrote: > >> +1 for ConfigurationBuilder >> >> -- Denis. >> Original Message >> From: Daniel Dekany >> Sent: Wednesday, 15 February 2017 10:49 >> To: Woonsan Ko >> Reply To: dev@freemarker.incubator.apache.org >> Subject: [FM3] Configuration mutability (Was: [FM3] Configuration API >> simplification) >> >> Wednesday, February 15, 2017, 12:16:12 AM, Woonsan Ko wrote: >> >>> Also, if possible, I would like to have an immutable object or >>> representation than the Configuration itself. For example, >>> template#getConfiguration doesn't have to return a mutable object, for >>> instance. But it requires a bigger change, I guess. >> >> A good point, that's the totally correct way of doing things. So we >> could have a mutable Configuration.Builder, which yields an immutable >> Configuration. We also have some ObjectWrapper, which is a bean in >> itself, but is also part of the Configuration. We certainly will have >> another such "injected bean", the TemplateResolver, and maybe some >> more on long term. Those also could follow this pattern (i.e, being >> immutable and only offer a builder). >> >> Question is, what extra pain that means for the average user. Like >> then in Spring XML you have to create a factory bean from Xxx.Builder, >> and then create yet another bean with factory-bean and factory-method >> attribute (because for some reason javax.annotation doesn't have >> anything like Spring FactoryBean, and freemarker-core can't depend on >> Spring obviously). Or, we can subclass all builders in something like >> freemarker-spring, to create a FactoryBean version of them, and then >> tell people to (a) pull in the depednecy and (b) use those classes >> instead of those shown in the core JavaDoc and in the Manual >> examples... So it's not very streamlined. If you have traditional >> beans, it's simpler for the users (especially if they don't have a >> PostConstruct either). Note sure which compromise is the winner. >> >> If Configuration (and the others) remains a bean, I want to throw an >> IllegalStateException if a the object were already "used" (like >> template processing was already invoked), and then someone calls >> cfg.setXxx(value). In FM2 we say that if you do that from multiple >> threads, then it won't work correctly, but we allow changing the >> Configuration at any time if you are only using it from the same >> thread. That's a possibility I want to remove, even if we don't switch >> to builders. >> >> Oh, and yet another complication with builders... If Configuration is >> immutable, then it only will have getters, no setters. But, >> Environment is mutable with setters, and it should be. Both >> Configuration and Environment extends Configurable. So it seems now we >> should split that class into a getters-only class and a subclass that >> adds setters... (That might will be necessary because that Template is >> mutable with setters is not all right either. Or we can just throw >> IllegalStateException there too.) >> > > -- > Thanks, > Daniel Dekany >
Re: [FM3] Configuration mutability (Was: [FM3] Configuration API simplification)
On Wed, Feb 15, 2017 at 2:54 PM, Woonsan Ko wrote:
> On Wed, Feb 15, 2017 at 5:48 AM, Daniel Dekany wrote:
>> Wednesday, February 15, 2017, 12:16:12 AM, Woonsan Ko wrote:
>>
>>> Also, if possible, I would like to have an immutable object or
>>> representation than the Configuration itself. For example,
>>> template#getConfiguration doesn't have to return a mutable object, for
>>> instance. But it requires a bigger change, I guess.
>>
>> A good point, that's the totally correct way of doing things. So we
>> could have a mutable Configuration.Builder, which yields an immutable
>> Configuration. We also have some ObjectWrapper, which is a bean in
>> itself, but is also part of the Configuration. We certainly will have
>> another such "injected bean", the TemplateResolver, and maybe some
>> more on long term. Those also could follow this pattern (i.e, being
>> immutable and only offer a builder).
>>
>> Question is, what extra pain that means for the average user. Like
>> then in Spring XML you have to create a factory bean from Xxx.Builder,
>> and then create yet another bean with factory-bean and factory-method
>> attribute (because for some reason javax.annotation doesn't have
>> anything like Spring FactoryBean, and freemarker-core can't depend on
>> Spring obviously). Or, we can subclass all builders in something like
>> freemarker-spring, to create a FactoryBean version of them, and then
>> tell people to (a) pull in the depednecy and (b) use those classes
>> instead of those shown in the core JavaDoc and in the Manual
>> examples... So it's not very streamlined. If you have traditional
>> beans, it's simpler for the users (especially if they don't have a
>> PostConstruct either). Note sure which compromise is the winner.
>
> From spring users' perspective, I don't think it would be a problem
> for them to add one more dependency on freemarker-spring to use the
> factory beans (dedicatedly designed for spring). And, they have
> already been doing it with others (JndiObjectFactoryBean,
> MBeanServerFactoryBean, FreemarkerConfigurationFactoryBean, etc.).
> It might be a little bit more work from development perspective, but I
> don't see anything weird from end users' perspective by having
> freemarker-spring and FactobyBean layers.
> Also, in general, I think it's a lot safer and more resilient in
> upgrade paths to use a factory bean instead of allowing to access
> mutable bean directly because we can safely control everything through
> the factory.
>
> Anyway, I like the idea having Builders which could perhaps look like
> a fluent Java DSL. For example,
BTW, I'm not suggesting we should prefer fluent style API everywhere
(I'm kind of against changing everything to fluent style without good
reasons), but I'm just saying new Builders could be designed with that
style in mind.
Regards,
Woonsan
>
> Configuration config = ConfigBuilder.create(props) // ConfigBuilder
> or Config.Builder
> .templateUpdateDelayMilliseconds(6)
> .defaultEncoding("UTF-8")
> //...
> .build();
>
> I think this will help people using Java API directly. And, I think
> it's also a good idea to have FactoryBeans (using the Builder APIs) in
> freemarker-spring for their convenience.
>
>>
>> If Configuration (and the others) remains a bean, I want to throw an
>> IllegalStateException if a the object were already "used" (like
>> template processing was already invoked), and then someone calls
>> cfg.setXxx(value). In FM2 we say that if you do that from multiple
>> threads, then it won't work correctly, but we allow changing the
>> Configuration at any time if you are only using it from the same
>> thread. That's a possibility I want to remove, even if we don't switch
>> to builders.
>
> Runtime exception option sounds like less attractive to me than
> factory beans. I would happily accept freemarker-spring library to
> take advantage of all the good factory beans.
>
>>
>> Oh, and yet another complication with builders... If Configuration is
>> immutable, then it only will have getters, no setters. But,
>> Environment is mutable with setters, and it should be. Both
>> Configuration and Environment extends Configurable. So it seems now we
>> should split that class into a getters-only class and a subclass that
>> adds setters... (That might will be necessary because that Template is
>> mutable with setters is not all right either. Or we can just throw
>> IllegalStateException there too.)
>
> Yeah, I would separate those.
>
> Regards,
>
> Woonsan
>
>>
>> --
>> Thanks,
>> Daniel Dekany
>>
Re: [FM3] Configuration mutability (Was: [FM3] Configuration API simplification)
On Wed, Feb 15, 2017 at 5:48 AM, Daniel Dekany wrote:
> Wednesday, February 15, 2017, 12:16:12 AM, Woonsan Ko wrote:
>
>> Also, if possible, I would like to have an immutable object or
>> representation than the Configuration itself. For example,
>> template#getConfiguration doesn't have to return a mutable object, for
>> instance. But it requires a bigger change, I guess.
>
> A good point, that's the totally correct way of doing things. So we
> could have a mutable Configuration.Builder, which yields an immutable
> Configuration. We also have some ObjectWrapper, which is a bean in
> itself, but is also part of the Configuration. We certainly will have
> another such "injected bean", the TemplateResolver, and maybe some
> more on long term. Those also could follow this pattern (i.e, being
> immutable and only offer a builder).
>
> Question is, what extra pain that means for the average user. Like
> then in Spring XML you have to create a factory bean from Xxx.Builder,
> and then create yet another bean with factory-bean and factory-method
> attribute (because for some reason javax.annotation doesn't have
> anything like Spring FactoryBean, and freemarker-core can't depend on
> Spring obviously). Or, we can subclass all builders in something like
> freemarker-spring, to create a FactoryBean version of them, and then
> tell people to (a) pull in the depednecy and (b) use those classes
> instead of those shown in the core JavaDoc and in the Manual
> examples... So it's not very streamlined. If you have traditional
> beans, it's simpler for the users (especially if they don't have a
> PostConstruct either). Note sure which compromise is the winner.
>From spring users' perspective, I don't think it would be a problem
for them to add one more dependency on freemarker-spring to use the
factory beans (dedicatedly designed for spring). And, they have
already been doing it with others (JndiObjectFactoryBean,
MBeanServerFactoryBean, FreemarkerConfigurationFactoryBean, etc.).
It might be a little bit more work from development perspective, but I
don't see anything weird from end users' perspective by having
freemarker-spring and FactobyBean layers.
Also, in general, I think it's a lot safer and more resilient in
upgrade paths to use a factory bean instead of allowing to access
mutable bean directly because we can safely control everything through
the factory.
Anyway, I like the idea having Builders which could perhaps look like
a fluent Java DSL. For example,
Configuration config = ConfigBuilder.create(props) // ConfigBuilder
or Config.Builder
.templateUpdateDelayMilliseconds(6)
.defaultEncoding("UTF-8")
//...
.build();
I think this will help people using Java API directly. And, I think
it's also a good idea to have FactoryBeans (using the Builder APIs) in
freemarker-spring for their convenience.
>
> If Configuration (and the others) remains a bean, I want to throw an
> IllegalStateException if a the object were already "used" (like
> template processing was already invoked), and then someone calls
> cfg.setXxx(value). In FM2 we say that if you do that from multiple
> threads, then it won't work correctly, but we allow changing the
> Configuration at any time if you are only using it from the same
> thread. That's a possibility I want to remove, even if we don't switch
> to builders.
Runtime exception option sounds like less attractive to me than
factory beans. I would happily accept freemarker-spring library to
take advantage of all the good factory beans.
>
> Oh, and yet another complication with builders... If Configuration is
> immutable, then it only will have getters, no setters. But,
> Environment is mutable with setters, and it should be. Both
> Configuration and Environment extends Configurable. So it seems now we
> should split that class into a getters-only class and a subclass that
> adds setters... (That might will be necessary because that Template is
> mutable with setters is not all right either. Or we can just throw
> IllegalStateException there too.)
Yeah, I would separate those.
Regards,
Woonsan
>
> --
> Thanks,
> Daniel Dekany
>
Re: [FM3] Configuration mutability (Was: [FM3] Configuration API simplification)
BTW, Configuration.Builder or ConfigurationBuilder? Both convention exists in the wild. I have recently noticed that the Java API is using Xxx.Builder at some places, so maybe that will be the more popular convention. It surely has the advantage that it can call a private constructor of the built class (or access other private members of it). Wednesday, February 15, 2017, 12:30:30 PM, brede...@me.com wrote: > +1 for ConfigurationBuilder > > -- Denis. > Original Message > From: Daniel Dekany > Sent: Wednesday, 15 February 2017 10:49 > To: Woonsan Ko > Reply To: dev@freemarker.incubator.apache.org > Subject: [FM3] Configuration mutability (Was: [FM3] Configuration API > simplification) > > Wednesday, February 15, 2017, 12:16:12 AM, Woonsan Ko wrote: > >> Also, if possible, I would like to have an immutable object or >> representation than the Configuration itself. For example, >> template#getConfiguration doesn't have to return a mutable object, for >> instance. But it requires a bigger change, I guess. > > A good point, that's the totally correct way of doing things. So we > could have a mutable Configuration.Builder, which yields an immutable > Configuration. We also have some ObjectWrapper, which is a bean in > itself, but is also part of the Configuration. We certainly will have > another such "injected bean", the TemplateResolver, and maybe some > more on long term. Those also could follow this pattern (i.e, being > immutable and only offer a builder). > > Question is, what extra pain that means for the average user. Like > then in Spring XML you have to create a factory bean from Xxx.Builder, > and then create yet another bean with factory-bean and factory-method > attribute (because for some reason javax.annotation doesn't have > anything like Spring FactoryBean, and freemarker-core can't depend on > Spring obviously). Or, we can subclass all builders in something like > freemarker-spring, to create a FactoryBean version of them, and then > tell people to (a) pull in the depednecy and (b) use those classes > instead of those shown in the core JavaDoc and in the Manual > examples... So it's not very streamlined. If you have traditional > beans, it's simpler for the users (especially if they don't have a > PostConstruct either). Note sure which compromise is the winner. > > If Configuration (and the others) remains a bean, I want to throw an > IllegalStateException if a the object were already "used" (like > template processing was already invoked), and then someone calls > cfg.setXxx(value). In FM2 we say that if you do that from multiple > threads, then it won't work correctly, but we allow changing the > Configuration at any time if you are only using it from the same > thread. That's a possibility I want to remove, even if we don't switch > to builders. > > Oh, and yet another complication with builders... If Configuration is > immutable, then it only will have getters, no setters. But, > Environment is mutable with setters, and it should be. Both > Configuration and Environment extends Configurable. So it seems now we > should split that class into a getters-only class and a subclass that > adds setters... (That might will be necessary because that Template is > mutable with setters is not all right either. Or we can just throw > IllegalStateException there too.) > -- Thanks, Daniel Dekany
Re: Pluggable template retrival/caching logic (Was: Template Loader Implementation that loads from a database (DataSource))
An interesting consequence of introducing the TemplateResolver concept
is that it the earlier top-level configuration settings, like
templateLoader, templateLookupStrategy, templateNameFormat,
cacheStorage, and templateUpdateDelayMillisecond should now be the
settings (JavaBean properties) of DefaultTemplateResolver, not of
Configuration. In theory at least. It's the same things as with
ObjectWrapper, where multiple implementations are possible, and each
has different settings.
This has some inconvenient consequences for the majority who uses the
DefaultTemplateResolver. Earlier, you have written:
cfg.setTemplateLoader(new FooTemplateLoader())
cfg.setTemplateUpdateDelayMilliseconds(10_000);
but now you had to write either:
DefaultTemplateResolver dtr = new DefaultTemplateResolver();
dtr.setTemplateLoader(new FooTemplateLoader());
dtr.setTemplateUpdateDelayMilliseconds(10_000);
cfg.setTemplateResolver(dtr);
or alternatively:
DefaultTemplateResolver dtr = (DefaultTemplateResolver)
dtr.getTemplateResolver();
dtr.setTemplateLoader(new FooTemplateLoader());
dtr.setTemplateUpdateDelayMilliseconds(10_000);
Also configuring with j.u.Properties becomes different, because you
can't have this anymore:
templateLoader = com.example.FooTemplateLoader
templateUpdateDelay = 10s
I guess instead it should be something like:
templateResolver.templateLoader = com.example.FooTemplateLoader
templateResolver.templateUpdateDelay = 10s
which is a syntax that we don't yet support (dotted setting names).
This syntax we do support though (in FM2):
templateResolver=DefaultTemplateResolver( \
templateLoader = com.example.FooTemplateLoader, \
templateUpdateDelayMilliseconds = 10_000 \
)
but it has the problem that you can't, for example, set the
templateUpdateDelay without also specifying the whole templateResolver
with all of its settings. So that's where the dotted notation is
better. OTOH for some other things the above syntax is better, like
for specifying the templateConfigurations (see
http://freemarker.org/docs/pgui_config_templateconfigurations.html).
So I think we should have both, and since we must ensure that
templateResolver=com.example.MyTemplateResolver is "executed" earlier
than templateResolver.mySetting=foo anyway, if mySetting is specified
on both places (i.e., you have
templateResolver=com.example.MyTemplateResolver(mySetting="foo")
there), then obviously templateResolver.mySetting wins.
And if we support the dotted notation in j.u.Properties, that should
work for the objectWrapper setting too, naturally.
Tuesday, February 7, 2017, 11:59:08 AM, Daniel Dekany wrote:
> Tuesday, February 7, 2017, 7:40:18 AM, David E Jones wrote:
>
>> On Tue, 2017-01-24 at 23:47 +0100, Daniel Dekany wrote:
>>>
>>> Should the TemplateResolver-s (the work name for the new interface)
>>> still get the template name already resolved to absolute and
>>> normalized? So when cfg.getTemplate is called, then it invokes
>>> cfg.templateNameFormat.normalizeAbsoluteName(String) to normalize the
>>> path, and only then it calls the TemplateResolver. Also #import and
>>> such currently calls templateNameFormat.toAbsoluteName(String,
>>> String), and that would happen earlier than TemplateResolver is
>>> involved.
>>>
>>> And how would it look... perhaps like this:
>>>
>>> public interface TemplateResolver {
>>>
>>> Template getTemplate(String templatePath, Local lookupLocale, Object
>>> customLookupCondition)
>>> throws IOException;
>>>
>>> }
>>>
>>> You may notice that it misses the `encoding` and `parsed` parameter of
>>> cfg.getTemplate. That's because I belive that in FM3 we should not
>>> allow specifying those as getTemplate parameters anymore, but that
>>> will be another thread when we get there.
>>
>> IMO it should pass through the exact text from the include/etc in
>> the template. It might be worth constraining to a valid URI syntax
>> but other than that a TemplateResolver would be much more flexible if no
>> normalization/etc is done.
>
> In FM2 there's a class like this:
>
> public abstract class TemplateNameFormat {
> ...
> abstract String toAbsoluteName(String baseName, String
> targetName) throws MalformedTemplateNameException;
> abstract String normalizeAbsoluteName(String name) throws
> MalformedTemplateNameException;
> }
>
> My idea was that if we allow users to provide a custom implementation,
> then if the standard rules don't fit your custom TemplateResolver, you
> just create a custom TemplateNameFormat too. But now that I think
> about it more, pushing the TemplateNameFormat functionality on the
> TemplateResolver implementation does indeed look like a better
> approach, because then it can't happen that a configuration
> accidentally contains a TemplateResolver with a TemplateNameFormat
> that doesn't work well with it. But then the above two methods should
> be part of the TemplateResolver interface (because, for exa
Re: [FM3] Configuration mutability (Was: [FM3] Configuration API simplification)
+1 for ConfigurationBuilder -- Denis. Original Message From: Daniel Dekany Sent: Wednesday, 15 February 2017 10:49 To: Woonsan Ko Reply To: dev@freemarker.incubator.apache.org Subject: [FM3] Configuration mutability (Was: [FM3] Configuration API simplification) Wednesday, February 15, 2017, 12:16:12 AM, Woonsan Ko wrote: > Also, if possible, I would like to have an immutable object or > representation than the Configuration itself. For example, > template#getConfiguration doesn't have to return a mutable object, for > instance. But it requires a bigger change, I guess. A good point, that's the totally correct way of doing things. So we could have a mutable Configuration.Builder, which yields an immutable Configuration. We also have some ObjectWrapper, which is a bean in itself, but is also part of the Configuration. We certainly will have another such "injected bean", the TemplateResolver, and maybe some more on long term. Those also could follow this pattern (i.e, being immutable and only offer a builder). Question is, what extra pain that means for the average user. Like then in Spring XML you have to create a factory bean from Xxx.Builder, and then create yet another bean with factory-bean and factory-method attribute (because for some reason javax.annotation doesn't have anything like Spring FactoryBean, and freemarker-core can't depend on Spring obviously). Or, we can subclass all builders in something like freemarker-spring, to create a FactoryBean version of them, and then tell people to (a) pull in the depednecy and (b) use those classes instead of those shown in the core JavaDoc and in the Manual examples... So it's not very streamlined. If you have traditional beans, it's simpler for the users (especially if they don't have a PostConstruct either). Note sure which compromise is the winner. If Configuration (and the others) remains a bean, I want to throw an IllegalStateException if a the object were already "used" (like template processing was already invoked), and then someone calls cfg.setXxx(value). In FM2 we say that if you do that from multiple threads, then it won't work correctly, but we allow changing the Configuration at any time if you are only using it from the same thread. That's a possibility I want to remove, even if we don't switch to builders. Oh, and yet another complication with builders... If Configuration is immutable, then it only will have getters, no setters. But, Environment is mutable with setters, and it should be. Both Configuration and Environment extends Configurable. So it seems now we should split that class into a getters-only class and a subclass that adds setters... (That might will be necessary because that Template is mutable with setters is not all right either. Or we can just throw IllegalStateException there too.) -- Thanks, Daniel Dekany
Re: Remove Version and incompatible improvements? (was [FM3] Configuration API simplification)
Wednesday, February 15, 2017, 12:43:31 AM, David E Jones wrote: > On Tue, 2017-02-14 at 21:32 +0100, Daniel Dekany wrote: >> I propose that in FM3 Configuration should only have two constructors: >> >> Configuration(Version incompatibleImprovements) >> Configuration(Properties properties) >> >> and that it should not have a setIncompatibleImprovements(Version) >> method. Because then it can be ensured that >> cfg.incompatibleImprovements is always set first, and not modified >> anymore. It's simplifies life considerably, because changing the >> incompatibleImprovements changes the default of some settings, so when >> in FM2 you change it at some later point, we will have to figure out >> which settings were never set with the public setter method, and thus >> still hold the initial default (not just something that's identical to >> it!), and thus has to be changed. Because Properties are esentially >> unordered, we also have to get all of them at once, so that we can >> sort them (bring incompatibleImprovements to the first place, sort any >> other dependent settings too). > > Not allowing configuration changes after init is very reasonable. According the above we would still allow that... you can call cfg.setXxx() anytime. But I have started a new thread about Configuration mutability. > I don't know about others, but for my part I wouldn't miss the > whole Version and incompatible improvements concept if it went away. > > Most libraries these days use semantic versioning > (major.minor.patch) and release notes to communicate when changes are not > backward > compatible (usually a major version bump, with minor versions for > backward compatible new features and patch versions for just bug > fixes). Along with this there is typically a branch for each major > or even minor version so that backported or direct fixes in > previous releases can just go into that code base. > > Supporting just one version per code branch (the latest being in > the trunk) would significantly simplify the code and avoid runtime > overhead, even if it is just in template parsing (not sure this is > an issue in any way now, just a potential one that crossed my > mind). > > There is always the issue of maintenance of previous release > branches, but this might actually be easier than basically supporting a > spec for each Version in the same code base. It would definitely be > easier for more contributors to get involved with the trunk for > new features as well as with release branches for maintenance and bug fixes. I think other projects get away without something like incompatibleImprovements be breaking backward compatibility more often. Note sure I want to push that on the users. So I would leave the infrastructure on the place, and if we don't use it (or not much), then hopefully it won't increate code complexity significantly. And ideally we won't need it, because we are careful and all, but that needs lots of eyes and mistakes do slip in. And on the day it will be needed, the architecture will be ready for it. > -David -- Thanks, Daniel Dekany
[FM3] Configuration mutability (Was: [FM3] Configuration API simplification)
Wednesday, February 15, 2017, 12:16:12 AM, Woonsan Ko wrote: > Also, if possible, I would like to have an immutable object or > representation than the Configuration itself. For example, > template#getConfiguration doesn't have to return a mutable object, for > instance. But it requires a bigger change, I guess. A good point, that's the totally correct way of doing things. So we could have a mutable Configuration.Builder, which yields an immutable Configuration. We also have some ObjectWrapper, which is a bean in itself, but is also part of the Configuration. We certainly will have another such "injected bean", the TemplateResolver, and maybe some more on long term. Those also could follow this pattern (i.e, being immutable and only offer a builder). Question is, what extra pain that means for the average user. Like then in Spring XML you have to create a factory bean from Xxx.Builder, and then create yet another bean with factory-bean and factory-method attribute (because for some reason javax.annotation doesn't have anything like Spring FactoryBean, and freemarker-core can't depend on Spring obviously). Or, we can subclass all builders in something like freemarker-spring, to create a FactoryBean version of them, and then tell people to (a) pull in the depednecy and (b) use those classes instead of those shown in the core JavaDoc and in the Manual examples... So it's not very streamlined. If you have traditional beans, it's simpler for the users (especially if they don't have a PostConstruct either). Note sure which compromise is the winner. If Configuration (and the others) remains a bean, I want to throw an IllegalStateException if a the object were already "used" (like template processing was already invoked), and then someone calls cfg.setXxx(value). In FM2 we say that if you do that from multiple threads, then it won't work correctly, but we allow changing the Configuration at any time if you are only using it from the same thread. That's a possibility I want to remove, even if we don't switch to builders. Oh, and yet another complication with builders... If Configuration is immutable, then it only will have getters, no setters. But, Environment is mutable with setters, and it should be. Both Configuration and Environment extends Configurable. So it seems now we should split that class into a getters-only class and a subclass that adds setters... (That might will be necessary because that Template is mutable with setters is not all right either. Or we can just throw IllegalStateException there too.) -- Thanks, Daniel Dekany
Re: [FM3] Configuration API simplification
Wednesday, February 15, 2017, 12:16:12 AM, Woonsan Ko wrote: > On Tue, Feb 14, 2017 at 3:32 PM, Daniel Dekany wrote: >> I propose that in FM3 Configuration should only have two constructors: >> >> Configuration(Version incompatibleImprovements) >> Configuration(Properties properties) > > Is it to make sure that users should set the version compatibility > explicitly by removing the default constructor? Yes. In the case of Configuration(Properties properties) my idea was that if the user sets the incompatibleImprovements in it, we can apply that first and in the constructor too. However, I realize that then it should be Configuration(Properties properties, Version defaultIncompatibleImprovements). Also, since then I start to think that the cfg.isXxxExplicitlySet() are useful even regardless of incompatibleImprovements, like they were utilized in FreemarkerServlet. Plus in Configurable (the superclass of Environment and Tempalte and Configuration) we need them to know when to fall back. And if we have the isXxxExplicitlySet-s after all, then most of the advantage of enforcing setting incompatibleImprovements first and only once is gone... > I thought it is a good option with the default constructor because > someone always want to use the latest default incompatible improvement > anyway with being willing to fix any incompatible template code. > Or could it have been dangerous sometimes? That would defeat the purpose of incompatibleImprovements. Adding a not entirely backward compatible fix would break backward compatibility, and so it couldn't be added (until and if ever something dramatic like FM4 happens). >> and that it should not have a setIncompatibleImprovements(Version) >> method. Because then it can be ensured that >> cfg.incompatibleImprovements is always set first, and not modified >> anymore. It's simplifies life considerably, because changing the >> incompatibleImprovements changes the default of some settings, so when >> in FM2 you change it at some later point, we will have to figure out >> which settings were never set with the public setter method, and thus >> still hold the initial default (not just something that's identical to >> it!), and thus has to be changed. Because Properties are esentially >> unordered, we also have to get all of them at once, so that we can >> sort them (bring incompatibleImprovements to the first place, sort any >> other dependent settings too). > > +1 > >> >> I also intend to remove `String getSetting(String)`, because it can't >> be implemented and has never worked. > > +1 > >> >> I'm also very inclined to remove setSetting(String) and >> setSettings(Properties), as it can lure the user into some ordering >> traps. I think Configuration(Properties properties) should be enough, >> and then, the user is forced to collect all the properties together >> (coming form different source maybe, that's why he collects them), and >> the we can do a proper ordering because we have all of them at once. > > +1 > >> >> Thoughts? > > I think any operations, which could affect or be affected by > invocation orders, should be removed. That will make the code cleaner > and safer. Since then I have doubts about my own somewhat impulsive proposal... Yes, taking out invocation order from the equation was a goal, but since then I realize that solving that for j.u.Properties-based configuring is not enough at all. More modern applications (frameworks) should use the setter methods (as in Spring IoC for example). But when you set JavaBean properties, do you used to care abut the order? Even if you can influence the order (though not sure what Spring does if you configure something with XML), I think usually you don't pay attention to that. So being strict about when and how j.u.Properties can be applied doesn't gets us far, as we have to be order independent with the setter anyway. > Also, if possible, I would like to have an immutable object or > representation than the Configuration itself. For example, > template#getConfiguration doesn't have to return a mutable object, for > instance. But it requires a bigger change, I guess. Yeah, I will respond to this in another thread. > Cheers, > > Woonsan > >> >> -- >> Thanks, >> Daniel Dekany >> > -- Thanks, Daniel Dekany
