Re: [FM3] Configuration mutability (Was: [FM3] Configuration API simplification)

[Daniel Dekany]

Thursday, May 11, 2017, 6:49:09 AM, Woonsan Ko wrote:

> Hi Daniel,
>
> On Fri, May 5, 2017 at 3:18 PM, Daniel Dekany  wrote:
>> This is now partially implemented (I have just committed it). There
>> are many rough edges yet, but Configuration is now immutable, and it
>> basically works. See:
>>
>>   
>> https://github.com/apache/incubator-freemarker/blob/3/src/test/java/org/apache/freemarker/manualtest/GettingStartedExample.java
>>
>> Would be good if more people looks at this thing. As you will see if
>> you do, there's a hierarchy of configuration related interfaces, which
>> are implemented by both XxxConfiguration and XxxConfiguration.Builder.
>> And so on... please find mistakes in this stuff, especially
>> architectural ones.
>
> It looks really good! Great work!

Speaking of the XxxConfiguration interfaces... If we want to support
specifying ObjectWrapper-s and such composite values with a builder
instead of with the final object (see earlier mails, partially still
quoted at end), then the builder classes can't implement the
XxxConfiguration interface, which would be quite a loss. They can't,
because for example, now we have
ProcessingConfiguration.getObjectWrapper(), but according the plan the
builder will only have
ProcessingConfiguration.getObjectWrapperBuilder(), and
setObjectWrapper(ObjectWrapper) is just a convenience method that
creates a dummy builder and delegates to
setObjectWrapperBuilder(ObjectWrapperBuilder). Given that I expect
this "layered configuration" thing to be a very rare demand, and
having it would complicate the public API for everyone (I couldn't see
this problem until I have implemented what we have now), I guess it
doesn't worth doing it after all. If someone will need this feature,
unfortunately, they will have to add their own builder facade that
postpones the creation of the ObjectWrapper (and such) from the
ObjectWrapperBuilder until it's know that ObjectWrapperBuilder won't
be modified anymore. (BTW, FreemarkerServlet got away without needing
this feature, so we are good there.)

>> There are some missing and somewhat undecided things, most notably:
>>
>> - If a setting has a Map or List value, when someone calls
>>   builder.setFoo(someMap), should we copy someMap, or let people
>>   modify it via aliasing? Well, at the very latest when
>>   builder.build() is called, we must copy it, as the result must be
>>   immutable. Also, aliasing has this problem that when the user
>>   utilizes it, it's potentially confusing for someone else reading the
>>   code later, as instinctively you only consider the content of the
>>   Map (or List, etc.) where builder.setFoo(someMap) was called. Also
>>   if you utilize aliasing like builder.getFoo().put(some, more), then
>>   you possibly unwillingly assume that the Map passed into setFoo was
>>   mutable (not a Guava ImmutableMao.of(...), which is popular). So I'm
>>   bending towards copying the map at the setter, and return an
>>   immutable view with builder.getFoo(), but add a dedicated
>>   `builder.putFoo(some, more)` (as it was in FM2 as well). This allows
>>   us to do some performance tricks, like cache invalidation in
>>   Environment.
>
> +1. Fully agree to all the pointers you made.

I will do this then, unless someone else has something to add.

>> - Should we allow calling builder.build() for multiple times? The
>>   trap one can run into with that is that then the two (or more)
>>   resulting objects will share the same stateful setting values, like
>>   the cacheStorage. That can lead to malfunction, and especially as
>>   cacheStorage has a default value, it's a quite sneaky thing
>>   (because, the user doesn't even realize that such setting exists, as
>>   he did not set it explicitly).
>
> I think a builder instance may return the same instance. I've read
> about the builder pattern again:
> https://en.wikipedia.org/wiki/Builder_pattern. There's no rule or
> reason to have to return a new instance on every #build() call. Even
> the Java example, CarBuilderImpl, in the page returns the same
> instance, too. It seems a lot safer to do in our context as well. So,
> I would return the same instance from the same builder on #build()
> calls.

But that's impossible if the builder creates immutable objects (which
is the main reason of using builders in many Java frameworks,
including FM3):

 Object obj1 = builder.build();
 b1.setFoo(123);
 Object obj2 = builder.build();
 assertSame(obj1, obj2)  // Impossible, as we create immutable objects

> Kind regards,
>
> Woonsan
>
>>
>>
>> Sunday, February 19, 2017, 7:41:54 PM, Woonsan Ko wrote:
>>
>>> On Sun, Feb 19, 2017 at 10:04 AM, Daniel Dekany  wrote:
 Sunday, February 19, 2017, 11:35:44 AM, Denis Bredelet wrote:

> My message was not clear, I mean builders are not required for
> standard configuration settings if they are simple types.

 Of course not. We are only talking about setting values like
 

Re: [FM3] Configuration mutability (Was: [FM3] Configuration API simplification)

[Woonsan Ko]

Hi Daniel,

On Fri, May 5, 2017 at 3:18 PM, Daniel Dekany  wrote:
> This is now partially implemented (I have just committed it). There
> are many rough edges yet, but Configuration is now immutable, and it
> basically works. See:
>
>   
> https://github.com/apache/incubator-freemarker/blob/3/src/test/java/org/apache/freemarker/manualtest/GettingStartedExample.java
>
> Would be good if more people looks at this thing. As you will see if
> you do, there's a hierarchy of configuration related interfaces, which
> are implemented by both XxxConfiguration and XxxConfiguration.Builder.
> And so on... please find mistakes in this stuff, especially
> architectural ones.

It looks really good! Great work!

>
> There are some missing and somewhat undecided things, most notably:
>
> - If a setting has a Map or List value, when someone calls
>   builder.setFoo(someMap), should we copy someMap, or let people
>   modify it via aliasing? Well, at the very latest when
>   builder.build() is called, we must copy it, as the result must be
>   immutable. Also, aliasing has this problem that when the user
>   utilizes it, it's potentially confusing for someone else reading the
>   code later, as instinctively you only consider the content of the
>   Map (or List, etc.) where builder.setFoo(someMap) was called. Also
>   if you utilize aliasing like builder.getFoo().put(some, more), then
>   you possibly unwillingly assume that the Map passed into setFoo was
>   mutable (not a Guava ImmutableMao.of(...), which is popular). So I'm
>   bending towards copying the map at the setter, and return an
>   immutable view with builder.getFoo(), but add a dedicated
>   `builder.putFoo(some, more)` (as it was in FM2 as well). This allows
>   us to do some performance tricks, like cache invalidation in
>   Environment.

+1. Fully agree to all the pointers you made.

>
> - Should we allow calling builder.build() for multiple times? The
>   trap one can run into with that is that then the two (or more)
>   resulting objects will share the same stateful setting values, like
>   the cacheStorage. That can lead to malfunction, and especially as
>   cacheStorage has a default value, it's a quite sneaky thing
>   (because, the user doesn't even realize that such setting exists, as
>   he did not set it explicitly).

I think a builder instance may return the same instance. I've read
about the builder pattern again:
https://en.wikipedia.org/wiki/Builder_pattern. There's no rule or
reason to have to return a new instance on every #build() call. Even
the Java example, CarBuilderImpl, in the page returns the same
instance, too. It seems a lot safer to do in our context as well. So,
I would return the same instance from the same builder on #build()
calls.

Kind regards,

Woonsan

>
>
> Sunday, February 19, 2017, 7:41:54 PM, Woonsan Ko wrote:
>
>> On Sun, Feb 19, 2017 at 10:04 AM, Daniel Dekany  wrote:
>>> Sunday, February 19, 2017, 11:35:44 AM, Denis Bredelet wrote:
>>>
 My message was not clear, I mean builders are not required for
 standard configuration settings if they are simple types.
>>>
>>> Of course not. We are only talking about setting values like
>>> ObjectWrapper, TemplateLoader, TemplateResolver (when it's added),
>>> etc. For them, I think we will have just bite the bullet and add both
>>> `void setFoo(Foo)`, and `void setFooBuilder(Builder)` (and their
>>> fluent API equivalent), and `Builder getFooBuilder()`, but no Foo
>>> getFoo().
>>
>> +1
>>
>>> And if you call setFoo(myFoo), then getFooBuilder() will
>>> return something like a `new o.a.f.core.ResultWrapperBuilder(myFoo)`,
>>> where ResultWrapperBuilder is just a wrapper myFoo, so that it
>>> satisfies the Builder interface.
>>
>> +1
>>
>>> I think I will also ban using
>>> setFoo(Foo) if Foo is known to have a Builder, just to enforce the
>>> intended usage pattern.
>>
>> +1
>>
>>> Unless somebody comes up with a better idea, I
>>> will try this approach when I get there, and we will see how it works
>>> out in reality.
>>
>> +1
>>
>> Woonsan
>>
>>>
> Le 19 févr. 2017 à 10:24, Denis Bredelet  a écrit :
>
> Hi Daniel,
>
 But then we lose the immutability of the built object, which was the
 point of using builders on the first place.
>>>
>>> Ah, you're totally right! If builder is able to update an immutable
>>> object, then the object turns out to be mutable indirectly through the
>>> builder. I missed that part. :-)
>>> Hmm.. it seems difficult to make it immutable considering those
>>> expensive nested objects.
>>> Would it be possible to introduce lifecycle methods such as
>>> #initialize() in Configuration to prohibit (IllegalStateException)
>>> using it and its nested objects until initialized?
>>
>> Sure, but that was plan B in case we don't go for builders at all (see
>> in the thread starter mail). I mean, if we have that, 

Re: [FM3] Configuration mutability (Was: [FM3] Configuration API simplification)

[Woonsan Ko]

On Sun, Feb 19, 2017 at 10:04 AM, Daniel Dekany  wrote:
> Sunday, February 19, 2017, 11:35:44 AM, Denis Bredelet wrote:
>
>> My message was not clear, I mean builders are not required for
>> standard configuration settings if they are simple types.
>
> Of course not. We are only talking about setting values like
> ObjectWrapper, TemplateLoader, TemplateResolver (when it's added),
> etc. For them, I think we will have just bite the bullet and add both
> `void setFoo(Foo)`, and `void setFooBuilder(Builder)` (and their
> fluent API equivalent), and `Builder getFooBuilder()`, but no Foo
> getFoo().

+1

> And if you call setFoo(myFoo), then getFooBuilder() will
> return something like a `new o.a.f.core.ResultWrapperBuilder(myFoo)`,
> where ResultWrapperBuilder is just a wrapper myFoo, so that it
> satisfies the Builder interface.

+1

> I think I will also ban using
> setFoo(Foo) if Foo is known to have a Builder, just to enforce the
> intended usage pattern.

+1

> Unless somebody comes up with a better idea, I
> will try this approach when I get there, and we will see how it works
> out in reality.

+1

Woonsan

>
>>> Le 19 févr. 2017 à 10:24, Denis Bredelet  a écrit :
>>>
>>> Hi Daniel,
>>>
>> But then we lose the immutability of the built object, which was the
>> point of using builders on the first place.
>
> Ah, you're totally right! If builder is able to update an immutable
> object, then the object turns out to be mutable indirectly through the
> builder. I missed that part. :-)
> Hmm.. it seems difficult to make it immutable considering those
> expensive nested objects.
> Would it be possible to introduce lifecycle methods such as
> #initialize() in Configuration to prohibit (IllegalStateException)
> using it and its nested objects until initialized?

 Sure, but that was plan B in case we don't go for builders at all (see
 in the thread starter mail). I mean, if we have that, what's the
 benefit of the builder classes? When I go for builders, I would hope
 fore real final fields in the built objects, and that also means that
 I don't have to worry about the ordering of the field assignments,
 because I got all everything at once, so I can sort the assignments.
>>>
>>> Final fields for built objects is not a requirement if all the standard 
>>> configuration settings are simple, e.g. String or Integer etc.
>>>
>>> Then possible mutations do not affect the standard configuration when using 
>>> a builder.
>>>
>>> — Denis.
>>>
> If so, perhaps we can wait for everything configured in each stage
> and finally initialize those expensive objects only once by the
> core. Also, would it be possible to wait until the final
> initialization phase, only capturing settings, without having to
> create a nested object such as ObjecrWrapper in earlier stage?

 Where do I capture the settings of the ObjectWrapper (the nested
 settings)? Anyway, that's why I thought that perhaps, if we have
 cfg.setFoo(T) where Foo is not just some primitive or such, then we
 also allow cfg.setFooBuilder(Builder)... and then, you
 can, using your words, capture the nested settings too.
>>>
>>
>>
>
> --
> Thanks,
>  Daniel Dekany
>

Re: [FM3] Configuration mutability (Was: [FM3] Configuration API simplification)

[Woonsan Ko]

On Sat, Feb 18, 2017 at 4:04 PM, Daniel Dekany  wrote:
> Saturday, February 18, 2017, 7:09:09 PM, Woonsan Ko wrote:
>
> [snip]
>> I see your point. Let me try for the plan A option with an example again. :-)
>> I guess it should look like this if we take all the existing requirements 
>> as-is:
>>
>> //stage#1:
>> ConfigurationFactory configFactory =
>> ConfigFactory.Builder.create(props)
>> .setObjectWrapperFactory(
>> DefaultObjectWrapperFactory.Builder.create()
>> .setIterableSupport(true)
>> //...
>> .build()
>> )
>> //...
>> .build();
>>
>> //stage#n:
>> configFactory.getObjectWrapperFactory()
>>  .setForceLegacyNonListCollections(true)
>>  // ...
>> Configuration config = configFactory
>> .setDefaultEncoding("UTF-8")
>> // ...
>> .newInstance()
>>
>> I've just tweaked the example with an additional Factories to build,
>> not the object directly. So, it's not creating the real Configuration
>> or ObjectWrapper, but the builder crates a new ConfigurationFactory or
>> ObjectWrapperFactory.
>> Could this work instead? Or do you think it's too complex?
> [snip]
>
> What's the practical difference between a builder and a factory in
> this case? I mean, I know what factories are for in general, but in
> this case, as far as I see, you have introduced a factory merely so
> that stage#n can tweak the settings. But then, isn't that factory just
> a builder? Then instead of set/getObjectWrapperFactory we had
> set/getObjectWrapperBuilder, and that's one less class (no need for
> XxxFactory).

Right. Either ObjectWrapperFactory or ObjectWrapperBuilder should work
without having two layers.
In Configuration example, I vaguely added ConfigurationFactory to be
created by ConfigurationFactory.Builder just in case the factory
itself could be determined by multiple arguments or which factory
could be determined by the builder, just as possibility.
If not, just having either Factory or Builder should suffice, IMHO.

Regards,

Woonsan

>
> --
> Thanks,
>  Daniel Dekany
>

Re: [FM3] Configuration mutability (Was: [FM3] Configuration API simplification)

[Daniel Dekany]

Saturday, February 18, 2017, 7:09:09 PM, Woonsan Ko wrote:

[snip]
> I see your point. Let me try for the plan A option with an example again. :-)
> I guess it should look like this if we take all the existing requirements 
> as-is:
>
> //stage#1:
> ConfigurationFactory configFactory =
> ConfigFactory.Builder.create(props)
> .setObjectWrapperFactory(
> DefaultObjectWrapperFactory.Builder.create()
> .setIterableSupport(true)
> //...
> .build()
> )
> //...
> .build();
>
> //stage#n:
> configFactory.getObjectWrapperFactory()
>  .setForceLegacyNonListCollections(true)
>  // ...
> Configuration config = configFactory
> .setDefaultEncoding("UTF-8")
> // ...
> .newInstance()
>
> I've just tweaked the example with an additional Factories to build,
> not the object directly. So, it's not creating the real Configuration
> or ObjectWrapper, but the builder crates a new ConfigurationFactory or
> ObjectWrapperFactory.
> Could this work instead? Or do you think it's too complex?
[snip]

What's the practical difference between a builder and a factory in
this case? I mean, I know what factories are for in general, but in
this case, as far as I see, you have introduced a factory merely so
that stage#n can tweak the settings. But then, isn't that factory just
a builder? Then instead of set/getObjectWrapperFactory we had
set/getObjectWrapperBuilder, and that's one less class (no need for
XxxFactory).

-- 
Thanks,
 Daniel Dekany

Re: [FM3] Configuration mutability (Was: [FM3] Configuration API simplification)

[Woonsan Ko]

On Fri, Feb 17, 2017 at 8:03 PM, Daniel Dekany  wrote:
> Friday, February 17, 2017, 11:48:34 PM, Woonsan Ko wrote:
>
>> On Fri, Feb 17, 2017 at 4:09 PM, Daniel Dekany  wrote:
>>> Friday, February 17, 2017, 3:16:28 PM, Woonsan Ko wrote:
>>>
 On Fri, Feb 17, 2017 at 4:22 AM, Daniel Dekany  wrote:
> Friday, February 17, 2017, 6:52:15 AM, Woonsan Ko wrote:
>
>> On Thu, Feb 16, 2017 at 1:28 PM, Daniel Dekany  
>> wrote:
>>> One other problem I see with builders is that they seem to be
>>> difficult to work with when it comes to working out the Configuration
>>> setting values in multiple stages.
>>>
>>> What do I mean by multiple stages? For example, the user specifies
>>> some settings in the web.xml in the case of FreemarkeServlet, so
>>> applying those is stage one. Then the Configuration builder is worked
>>> on by FreemarkeServlet itself, which adjusts it further, usually by
>>> setting some defaults where the setting wasn't set in the previous
>>> stage. Then it goes down one more stage, to FreeMarker itself, where
>>> the FreeMarker defined defaults are applied to the settings which were
>>> not set so far. You may say that it should be done on the opposite
>>> order, that is, FreeMarker fills its defaults, then FreemarkerServlet
>>> overwrites some setting with its own defaults, then comes the web.xml
>>> content. That's simpler to do, but has some problems. One is that you
>>> may construct setting values that will be later overwritten, and not
>>> all setting values are cheap to construct (unless... for complex value
>>> we require passing in a builder instead of the object). The other is
>>
>> Another option I'm thinking is to extend the Builder pattern to be
>> able to use an existing object instead of new creation when needed.
>> As an example pseudo code,
>>
>> // stage #1:
>>Configuration config = Config,Builder.create(props) // create new
>>.templateUpdateDelayMilliseconds(6)
>>.defaultEncoding("UTF-8")
>>//...
>>.build();
>>
>> // stage #2:
>> config = Config.Builder.update(config) // update existing config
>> .whitespaceStripping(true)
>> //...
>> .build();  // return the same config instance after updates
>>
>> // stage #n:
>>  // ...
>>
>>> that you may want to adjust settings nested inside values (such as the
>>> settings of an ObjectWrapper), so the top-level setting must be
>>> already set by the higher layer when it gets to you. Anyway, whichever
>>> order you chose, this is the use case.
>>
>> Nested one can also create or update (or createOrUpdate sometimes?), 
>> IMHO:
>>
>> // stage #1:
>>Configuration config = Config,Builder.create(props)
>>.objectWrapperBuilder(
>>// new builder to create a new objectWrapper
>>DefaultObjectWrapper.Builder.create()
>>.iterableSupport(true)
>>//...
>>.build()
>>)
>>//...
>>.build();
>>
>> // stage #2:
>> config = Config.Builder.update(config)
>>.objectWrapperBuilder(
>>// new builder to update existing objectWrapper
>>
>> DefaultObjectWrapper.Builder.update(config.getObjectWrapper())
>>.forceLegacyNonListCollections(true)
>>//...
>>.build()
>>)
>>//...
>>.build();
>
> That (recreate Builder from the built object) is a possibility. But
> note that it comes with some extra price over the obvious extra
> complexity it adds:
>
> - Objects are fully built, then dropped, then rebuilt... It almost never
>   matters in out case, but creating some setting values, like creating
>   some custom TemplateLoader-s or ObjectWrapper-s, can be relatively
>   expensive (like it involves I/O or permanent allocation of bigger
>   static data structures). So it's not an universally applicable
>   pattern at least.

 Oh, that's not what I meant.
 In the first example,

  // stage #2:
  config = Config.Builder.update(config) // update existing config
  .whitespaceStripping(true)
  //...
  .build();  // return the same config instance after updates

 the first line takes an existing Configuration object only to update
 and return it again only after setting more properties.
 So, the return is meaningless here, and #build() means 'updating
 build', not 'creation build', which I think can be regarded as an
 extended Builder pattern.
>>> [snip]
>>>
>>> But then we lose the immutability of the 

Re: [FM3] Configuration mutability (Was: [FM3] Configuration API simplification)

[Daniel Dekany]

Friday, February 17, 2017, 11:48:34 PM, Woonsan Ko wrote:

> On Fri, Feb 17, 2017 at 4:09 PM, Daniel Dekany  wrote:
>> Friday, February 17, 2017, 3:16:28 PM, Woonsan Ko wrote:
>>
>>> On Fri, Feb 17, 2017 at 4:22 AM, Daniel Dekany  wrote:
 Friday, February 17, 2017, 6:52:15 AM, Woonsan Ko wrote:

> On Thu, Feb 16, 2017 at 1:28 PM, Daniel Dekany  
> wrote:
>> One other problem I see with builders is that they seem to be
>> difficult to work with when it comes to working out the Configuration
>> setting values in multiple stages.
>>
>> What do I mean by multiple stages? For example, the user specifies
>> some settings in the web.xml in the case of FreemarkeServlet, so
>> applying those is stage one. Then the Configuration builder is worked
>> on by FreemarkeServlet itself, which adjusts it further, usually by
>> setting some defaults where the setting wasn't set in the previous
>> stage. Then it goes down one more stage, to FreeMarker itself, where
>> the FreeMarker defined defaults are applied to the settings which were
>> not set so far. You may say that it should be done on the opposite
>> order, that is, FreeMarker fills its defaults, then FreemarkerServlet
>> overwrites some setting with its own defaults, then comes the web.xml
>> content. That's simpler to do, but has some problems. One is that you
>> may construct setting values that will be later overwritten, and not
>> all setting values are cheap to construct (unless... for complex value
>> we require passing in a builder instead of the object). The other is
>
> Another option I'm thinking is to extend the Builder pattern to be
> able to use an existing object instead of new creation when needed.
> As an example pseudo code,
>
> // stage #1:
>Configuration config = Config,Builder.create(props) // create new
>.templateUpdateDelayMilliseconds(6)
>.defaultEncoding("UTF-8")
>//...
>.build();
>
> // stage #2:
> config = Config.Builder.update(config) // update existing config
> .whitespaceStripping(true)
> //...
> .build();  // return the same config instance after updates
>
> // stage #n:
>  // ...
>
>> that you may want to adjust settings nested inside values (such as the
>> settings of an ObjectWrapper), so the top-level setting must be
>> already set by the higher layer when it gets to you. Anyway, whichever
>> order you chose, this is the use case.
>
> Nested one can also create or update (or createOrUpdate sometimes?), IMHO:
>
> // stage #1:
>Configuration config = Config,Builder.create(props)
>.objectWrapperBuilder(
>// new builder to create a new objectWrapper
>DefaultObjectWrapper.Builder.create()
>.iterableSupport(true)
>//...
>.build()
>)
>//...
>.build();
>
> // stage #2:
> config = Config.Builder.update(config)
>.objectWrapperBuilder(
>// new builder to update existing objectWrapper
>
> DefaultObjectWrapper.Builder.update(config.getObjectWrapper())
>.forceLegacyNonListCollections(true)
>//...
>.build()
>)
>//...
>.build();

 That (recreate Builder from the built object) is a possibility. But
 note that it comes with some extra price over the obvious extra
 complexity it adds:

 - Objects are fully built, then dropped, then rebuilt... It almost never
   matters in out case, but creating some setting values, like creating
   some custom TemplateLoader-s or ObjectWrapper-s, can be relatively
   expensive (like it involves I/O or permanent allocation of bigger
   static data structures). So it's not an universally applicable
   pattern at least.
>>>
>>> Oh, that's not what I meant.
>>> In the first example,
>>>
>>>  // stage #2:
>>>  config = Config.Builder.update(config) // update existing config
>>>  .whitespaceStripping(true)
>>>  //...
>>>  .build();  // return the same config instance after updates
>>>
>>> the first line takes an existing Configuration object only to update
>>> and return it again only after setting more properties.
>>> So, the return is meaningless here, and #build() means 'updating
>>> build', not 'creation build', which I think can be regarded as an
>>> extended Builder pattern.
>> [snip]
>>
>> But then we lose the immutability of the built object, which was the
>> point of using builders on the first place.
>
> Ah, you're totally right! If builder is able to update an immutable
> object, then the object turns out to be mutable 

Re: [FM3] Configuration mutability (Was: [FM3] Configuration API simplification)

[Woonsan Ko]

On Fri, Feb 17, 2017 at 4:09 PM, Daniel Dekany  wrote:
> Friday, February 17, 2017, 3:16:28 PM, Woonsan Ko wrote:
>
>> On Fri, Feb 17, 2017 at 4:22 AM, Daniel Dekany  wrote:
>>> Friday, February 17, 2017, 6:52:15 AM, Woonsan Ko wrote:
>>>
 On Thu, Feb 16, 2017 at 1:28 PM, Daniel Dekany  wrote:
> One other problem I see with builders is that they seem to be
> difficult to work with when it comes to working out the Configuration
> setting values in multiple stages.
>
> What do I mean by multiple stages? For example, the user specifies
> some settings in the web.xml in the case of FreemarkeServlet, so
> applying those is stage one. Then the Configuration builder is worked
> on by FreemarkeServlet itself, which adjusts it further, usually by
> setting some defaults where the setting wasn't set in the previous
> stage. Then it goes down one more stage, to FreeMarker itself, where
> the FreeMarker defined defaults are applied to the settings which were
> not set so far. You may say that it should be done on the opposite
> order, that is, FreeMarker fills its defaults, then FreemarkerServlet
> overwrites some setting with its own defaults, then comes the web.xml
> content. That's simpler to do, but has some problems. One is that you
> may construct setting values that will be later overwritten, and not
> all setting values are cheap to construct (unless... for complex value
> we require passing in a builder instead of the object). The other is

 Another option I'm thinking is to extend the Builder pattern to be
 able to use an existing object instead of new creation when needed.
 As an example pseudo code,

 // stage #1:
Configuration config = Config,Builder.create(props) // create new
.templateUpdateDelayMilliseconds(6)
.defaultEncoding("UTF-8")
//...
.build();

 // stage #2:
 config = Config.Builder.update(config) // update existing config
 .whitespaceStripping(true)
 //...
 .build();  // return the same config instance after updates

 // stage #n:
  // ...

> that you may want to adjust settings nested inside values (such as the
> settings of an ObjectWrapper), so the top-level setting must be
> already set by the higher layer when it gets to you. Anyway, whichever
> order you chose, this is the use case.

 Nested one can also create or update (or createOrUpdate sometimes?), IMHO:

 // stage #1:
Configuration config = Config,Builder.create(props)
.objectWrapperBuilder(
// new builder to create a new objectWrapper
DefaultObjectWrapper.Builder.create()
.iterableSupport(true)
//...
.build()
)
//...
.build();

 // stage #2:
 config = Config.Builder.update(config)
.objectWrapperBuilder(
// new builder to update existing objectWrapper

 DefaultObjectWrapper.Builder.update(config.getObjectWrapper())
.forceLegacyNonListCollections(true)
//...
.build()
)
//...
.build();
>>>
>>> That (recreate Builder from the built object) is a possibility. But
>>> note that it comes with some extra price over the obvious extra
>>> complexity it adds:
>>>
>>> - Objects are fully built, then dropped, then rebuilt... It almost never
>>>   matters in out case, but creating some setting values, like creating
>>>   some custom TemplateLoader-s or ObjectWrapper-s, can be relatively
>>>   expensive (like it involves I/O or permanent allocation of bigger
>>>   static data structures). So it's not an universally applicable
>>>   pattern at least.
>>
>> Oh, that's not what I meant.
>> In the first example,
>>
>>  // stage #2:
>>  config = Config.Builder.update(config) // update existing config
>>  .whitespaceStripping(true)
>>  //...
>>  .build();  // return the same config instance after updates
>>
>> the first line takes an existing Configuration object only to update
>> and return it again only after setting more properties.
>> So, the return is meaningless here, and #build() means 'updating
>> build', not 'creation build', which I think can be regarded as an
>> extended Builder pattern.
> [snip]
>
> But then we lose the immutability of the built object, which was the
> point of using builders on the first place.

Ah, you're totally right! If builder is able to update an immutable
object, then the object turns out to be mutable indirectly through the
builder. I missed that part. :-)
Hmm.. it seems difficult to make it immutable considering those
expensive nested objects.
Would it be possible to 

Re: [FM3] Configuration mutability (Was: [FM3] Configuration API simplification)

[Daniel Dekany]

Friday, February 17, 2017, 3:16:28 PM, Woonsan Ko wrote:

> On Fri, Feb 17, 2017 at 4:22 AM, Daniel Dekany  wrote:
>> Friday, February 17, 2017, 6:52:15 AM, Woonsan Ko wrote:
>>
>>> On Thu, Feb 16, 2017 at 1:28 PM, Daniel Dekany  wrote:
 One other problem I see with builders is that they seem to be
 difficult to work with when it comes to working out the Configuration
 setting values in multiple stages.

 What do I mean by multiple stages? For example, the user specifies
 some settings in the web.xml in the case of FreemarkeServlet, so
 applying those is stage one. Then the Configuration builder is worked
 on by FreemarkeServlet itself, which adjusts it further, usually by
 setting some defaults where the setting wasn't set in the previous
 stage. Then it goes down one more stage, to FreeMarker itself, where
 the FreeMarker defined defaults are applied to the settings which were
 not set so far. You may say that it should be done on the opposite
 order, that is, FreeMarker fills its defaults, then FreemarkerServlet
 overwrites some setting with its own defaults, then comes the web.xml
 content. That's simpler to do, but has some problems. One is that you
 may construct setting values that will be later overwritten, and not
 all setting values are cheap to construct (unless... for complex value
 we require passing in a builder instead of the object). The other is
>>>
>>> Another option I'm thinking is to extend the Builder pattern to be
>>> able to use an existing object instead of new creation when needed.
>>> As an example pseudo code,
>>>
>>> // stage #1:
>>>Configuration config = Config,Builder.create(props) // create new
>>>.templateUpdateDelayMilliseconds(6)
>>>.defaultEncoding("UTF-8")
>>>//...
>>>.build();
>>>
>>> // stage #2:
>>> config = Config.Builder.update(config) // update existing config
>>> .whitespaceStripping(true)
>>> //...
>>> .build();  // return the same config instance after updates
>>>
>>> // stage #n:
>>>  // ...
>>>
 that you may want to adjust settings nested inside values (such as the
 settings of an ObjectWrapper), so the top-level setting must be
 already set by the higher layer when it gets to you. Anyway, whichever
 order you chose, this is the use case.
>>>
>>> Nested one can also create or update (or createOrUpdate sometimes?), IMHO:
>>>
>>> // stage #1:
>>>Configuration config = Config,Builder.create(props)
>>>.objectWrapperBuilder(
>>>// new builder to create a new objectWrapper
>>>DefaultObjectWrapper.Builder.create()
>>>.iterableSupport(true)
>>>//...
>>>.build()
>>>)
>>>//...
>>>.build();
>>>
>>> // stage #2:
>>> config = Config.Builder.update(config)
>>>.objectWrapperBuilder(
>>>// new builder to update existing objectWrapper
>>>
>>> DefaultObjectWrapper.Builder.update(config.getObjectWrapper())
>>>.forceLegacyNonListCollections(true)
>>>//...
>>>.build()
>>>)
>>>//...
>>>.build();
>>
>> That (recreate Builder from the built object) is a possibility. But
>> note that it comes with some extra price over the obvious extra
>> complexity it adds:
>>
>> - Objects are fully built, then dropped, then rebuilt... It almost never
>>   matters in out case, but creating some setting values, like creating
>>   some custom TemplateLoader-s or ObjectWrapper-s, can be relatively
>>   expensive (like it involves I/O or permanent allocation of bigger
>>   static data structures). So it's not an universally applicable
>>   pattern at least.
>
> Oh, that's not what I meant.
> In the first example,
>
>  // stage #2:
>  config = Config.Builder.update(config) // update existing config
>  .whitespaceStripping(true)
>  //...
>  .build();  // return the same config instance after updates
>
> the first line takes an existing Configuration object only to update
> and return it again only after setting more properties.
> So, the return is meaningless here, and #build() means 'updating
> build', not 'creation build', which I think can be regarded as an
> extended Builder pattern.
[snip]

But then we lose the immutability of the built object, which was the
point of using builders on the first place.

> Yeah, I'm fine with `T getFoo()' or `boolean isFoo()'. But if it
> follows a fluent style, it's already free from the JavaBeans
> conventions anyway.

Many frameworks/tools look for JavaBean properties. Like Spring IoC
for example, if you are using the XML configuration. They couldn't
get/set the properties through a fluent API.

> The exception in fluent Builders seems fine to me.

(What exception do you mean?)

> Regards,
>
> Woonsan
>
>>
>>> Kind regards,
>>>
>>> Woonsan

Re: [FM3] Configuration mutability (Was: [FM3] Configuration API simplification)

[Daniel Dekany]

Friday, February 17, 2017, 6:52:15 AM, Woonsan Ko wrote:

> On Thu, Feb 16, 2017 at 1:28 PM, Daniel Dekany  wrote:
>> One other problem I see with builders is that they seem to be
>> difficult to work with when it comes to working out the Configuration
>> setting values in multiple stages.
>>
>> What do I mean by multiple stages? For example, the user specifies
>> some settings in the web.xml in the case of FreemarkeServlet, so
>> applying those is stage one. Then the Configuration builder is worked
>> on by FreemarkeServlet itself, which adjusts it further, usually by
>> setting some defaults where the setting wasn't set in the previous
>> stage. Then it goes down one more stage, to FreeMarker itself, where
>> the FreeMarker defined defaults are applied to the settings which were
>> not set so far. You may say that it should be done on the opposite
>> order, that is, FreeMarker fills its defaults, then FreemarkerServlet
>> overwrites some setting with its own defaults, then comes the web.xml
>> content. That's simpler to do, but has some problems. One is that you
>> may construct setting values that will be later overwritten, and not
>> all setting values are cheap to construct (unless... for complex value
>> we require passing in a builder instead of the object). The other is
>
> Another option I'm thinking is to extend the Builder pattern to be
> able to use an existing object instead of new creation when needed.
> As an example pseudo code,
>
> // stage #1:
>Configuration config = Config,Builder.create(props) // create new
>.templateUpdateDelayMilliseconds(6)
>.defaultEncoding("UTF-8")
>//...
>.build();
>
> // stage #2:
> config = Config.Builder.update(config) // update existing config
> .whitespaceStripping(true)
> //...
> .build();  // return the same config instance after updates
>
> // stage #n:
>  // ...
>
>> that you may want to adjust settings nested inside values (such as the
>> settings of an ObjectWrapper), so the top-level setting must be
>> already set by the higher layer when it gets to you. Anyway, whichever
>> order you chose, this is the use case.
>
> Nested one can also create or update (or createOrUpdate sometimes?), IMHO:
>
> // stage #1:
>Configuration config = Config,Builder.create(props)
>.objectWrapperBuilder(
>// new builder to create a new objectWrapper
>DefaultObjectWrapper.Builder.create()
>.iterableSupport(true)
>//...
>.build()
>)
>//...
>.build();
>
> // stage #2:
> config = Config.Builder.update(config)
>.objectWrapperBuilder(
>// new builder to update existing objectWrapper
>   
> DefaultObjectWrapper.Builder.update(config.getObjectWrapper())
>.forceLegacyNonListCollections(true)
>//...
>.build()
>)
>//...
>.build();

That (recreate Builder from the built object) is a possibility. But
note that it comes with some extra price over the obvious extra
complexity it adds:

- Objects are fully built, then dropped, then rebuilt... It almost never
  matters in out case, but creating some setting values, like creating
  some custom TemplateLoader-s or ObjectWrapper-s, can be relatively
  expensive (like it involves I/O or permanent allocation of bigger
  static data structures). So it's not an universally applicable
  pattern at least.

- If we want to have the isXxxSet() methods, then that information has
  to be stored into the built object now, otherwise the Builder can't be
  recreated from it. So builder concerns seep into the class of the
  built object, which is somewhat ugly.

>> The problem is with the configuration setting values that are
>> themselves beans and was (potentially) built with a Builder
>> (ObjectWrapper-s is a good example). I pass the Configuration.Builder
>> to the next stage, but there you can't get the builder of the
>> ObjectWrapper anymore there, only the read-only ObjectWrapper. So you
>> can't customize the settings of that further. With other words, you
>> can't customize nested settings, only the top level ones, which
>> strikes me as a quite arbitrary restriction. Unless, I also allow
>> setting the value of a configuration setting to a builder object
>> instead of the value object itself... which starts to be
>> overcomplicated, because you can now set the same setting either by
>> specifying a builder or the result object. Not sure if that's
>> acceptable.
>
> Can we perhaps consider to extend Builder pattern to be able to either
> create or update like the examples above?

We can (see earlier), but the whole thing starts to be rather
overcomplicated. Maybe I should just give up multi-step configuring...

>> BTW, with Builder-s you are lucky if you can get back the property
>> values that you or someone else has set earlier at all. Many 

Re: [FM3] Configuration mutability (Was: [FM3] Configuration API simplification)

[Woonsan Ko]

On Thu, Feb 16, 2017 at 1:28 PM, Daniel Dekany  wrote:
> One other problem I see with builders is that they seem to be
> difficult to work with when it comes to working out the Configuration
> setting values in multiple stages.
>
> What do I mean by multiple stages? For example, the user specifies
> some settings in the web.xml in the case of FreemarkeServlet, so
> applying those is stage one. Then the Configuration builder is worked
> on by FreemarkeServlet itself, which adjusts it further, usually by
> setting some defaults where the setting wasn't set in the previous
> stage. Then it goes down one more stage, to FreeMarker itself, where
> the FreeMarker defined defaults are applied to the settings which were
> not set so far. You may say that it should be done on the opposite
> order, that is, FreeMarker fills its defaults, then FreemarkerServlet
> overwrites some setting with its own defaults, then comes the web.xml
> content. That's simpler to do, but has some problems. One is that you
> may construct setting values that will be later overwritten, and not
> all setting values are cheap to construct (unless... for complex value
> we require passing in a builder instead of the object). The other is

Another option I'm thinking is to extend the Builder pattern to be
able to use an existing object instead of new creation when needed.
As an example pseudo code,

// stage #1:
   Configuration config = Config,Builder.create(props) // create new
   .templateUpdateDelayMilliseconds(6)
   .defaultEncoding("UTF-8")
   //...
   .build();

// stage #2:
config = Config.Builder.update(config) // update existing config
.whitespaceStripping(true)
//...
.build();  // return the same config instance after updates

// stage #n:
 // ...

> that you may want to adjust settings nested inside values (such as the
> settings of an ObjectWrapper), so the top-level setting must be
> already set by the higher layer when it gets to you. Anyway, whichever
> order you chose, this is the use case.

Nested one can also create or update (or createOrUpdate sometimes?), IMHO:

// stage #1:
   Configuration config = Config,Builder.create(props)
   .objectWrapperBuilder(
   // new builder to create a new objectWrapper
   DefaultObjectWrapper.Builder.create()
   .iterableSupport(true)
   //...
   .build()
   )
   //...
   .build();

// stage #2:
config = Config.Builder.update(config)
   .objectWrapperBuilder(
   // new builder to update existing objectWrapper
   DefaultObjectWrapper.Builder.update(config.getObjectWrapper())
   .forceLegacyNonListCollections(true)
   //...
   .build()
   )
   //...
   .build();

>
> The problem is with the configuration setting values that are
> themselves beans and was (potentially) built with a Builder
> (ObjectWrapper-s is a good example). I pass the Configuration.Builder
> to the next stage, but there you can't get the builder of the
> ObjectWrapper anymore there, only the read-only ObjectWrapper. So you
> can't customize the settings of that further. With other words, you
> can't customize nested settings, only the top level ones, which
> strikes me as a quite arbitrary restriction. Unless, I also allow
> setting the value of a configuration setting to a builder object
> instead of the value object itself... which starts to be
> overcomplicated, because you can now set the same setting either by
> specifying a builder or the result object. Not sure if that's
> acceptable.

Can we perhaps consider to extend Builder pattern to be able to either
create or update like the examples above?

>
> BTW, with Builder-s you are lucky if you can get back the property
> values that you or someone else has set earlier at all. Many thinks
> that they should be write-only objects. I think it's overly
> restrictive. So as far as I'm concerned, the builder should these
> methods for each setting foo (for a simple value this time):
> - void setFoo(T value)
> - Builder foo(T value) // Some prefer withFoo... I don't.
> - T getFoo() // Design pattern junkies raise eyebrows here ;)
> - boolean isFooSet() // This is how you know that you can supply the
>  // default. Called isFooExplicitlySet() in FM2.

I have preferred 'Builder foo(T value)' and 'T foo()' simply, but I'll
all ears. ;-)

Kind regards,

Woonsan

>
> Any thoughts? Especially on what to do with those nested settings.
>
>
> Wednesday, February 15, 2017, 8:54:52 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

Re: [FM3] Configuration mutability (Was: [FM3] Configuration API simplification)

[Daniel Dekany]

One other problem I see with builders is that they seem to be
difficult to work with when it comes to working out the Configuration
setting values in multiple stages.

What do I mean by multiple stages? For example, the user specifies
some settings in the web.xml in the case of FreemarkeServlet, so
applying those is stage one. Then the Configuration builder is worked
on by FreemarkeServlet itself, which adjusts it further, usually by
setting some defaults where the setting wasn't set in the previous
stage. Then it goes down one more stage, to FreeMarker itself, where
the FreeMarker defined defaults are applied to the settings which were
not set so far. You may say that it should be done on the opposite
order, that is, FreeMarker fills its defaults, then FreemarkerServlet
overwrites some setting with its own defaults, then comes the web.xml
content. That's simpler to do, but has some problems. One is that you
may construct setting values that will be later overwritten, and not
all setting values are cheap to construct (unless... for complex value
we require passing in a builder instead of the object). The other is
that you may want to adjust settings nested inside values (such as the
settings of an ObjectWrapper), so the top-level setting must be
already set by the higher layer when it gets to you. Anyway, whichever
order you chose, this is the use case.

The problem is with the configuration setting values that are
themselves beans and was (potentially) built with a Builder
(ObjectWrapper-s is a good example). I pass the Configuration.Builder
to the next stage, but there you can't get the builder of the
ObjectWrapper anymore there, only the read-only ObjectWrapper. So you
can't customize the settings of that further. With other words, you
can't customize nested settings, only the top level ones, which
strikes me as a quite arbitrary restriction. Unless, I also allow
setting the value of a configuration setting to a builder object
instead of the value object itself... which starts to be
overcomplicated, because you can now set the same setting either by
specifying a builder or the result object. Not sure if that's
acceptable.

BTW, with Builder-s you are lucky if you can get back the property
values that you or someone else has set earlier at all. Many thinks
that they should be write-only objects. I think it's overly
restrictive. So as far as I'm concerned, the builder should these
methods for each setting foo (for a simple value this time):
- void setFoo(T value)
- Builder foo(T value) // Some prefer withFoo... I don't.
- T getFoo() // Design pattern junkies raise eyebrows here ;)
- boolean isFooSet() // This is how you know that you can supply the
 // default. Called isFooExplicitlySet() in FM2.

Any thoughts? Especially on what to do with those nested settings.


Wednesday, February 15, 2017, 8:54:52 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 

Re: [FM3] Configuration mutability (Was: [FM3] Configuration API simplification)

[Woonsan Ko]

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)

[Woonsan Ko]

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)

[Woonsan Ko]

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)

[bredelet]

+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