You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@directory.apache.org by Antoine Levy-Lambert <an...@gmx.de> on 2010/12/17 15:31:08 UTC
Re: svn commit: r1050378 - /directory/apacheds/trunk/server-config/src/main/resources/config.ldif
Hello Pierre-Alain,
is this change needed ?
I would think that these attributes are only needed when you are dealing
with for instance a multi-valued string.
But this is a list contained in a subentry ou=authenticators
So I think that this ads-authenticators attribute is not required.
And it is redundant with the other items below.
Regards,
Antoine
On 12/17/2010 8:32 AM, pamarcelot@apache.org wrote:
> Author: pamarcelot
> Date: Fri Dec 17 13:32:30 2010
> New Revision: 1050378
>
> URL: http://svn.apache.org/viewvc?rev=1050378&view=rev
> Log:
> Added missing attribute 'ads-authenticators'.
>
> Modified:
> directory/apacheds/trunk/server-config/src/main/resources/config.ldif
>
> Modified: directory/apacheds/trunk/server-config/src/main/resources/config.ldif
> URL: http://svn.apache.org/viewvc/directory/apacheds/trunk/server-config/src/main/resources/config.ldif?rev=1050378&r1=1050377&r2=1050378&view=diff
> ==============================================================================
> --- directory/apacheds/trunk/server-config/src/main/resources/config.ldif (original)
> +++ directory/apacheds/trunk/server-config/src/main/resources/config.ldif Fri Dec 17 13:32:30 2010
> @@ -78,6 +78,9 @@ ads-interceptororder: 2
> ads-interceptorclassname: org.apache.directory.server.core.authn.AuthenticationInterceptor
> ads-interceptorid: authenticationInterceptor
> ads-enabled: TRUE
> +ads-authenticators: anonymousauthenticator
> +ads-authenticators: simpleauthenticator
> +ads-authenticators: strongauthenticator
>
> dn: ou=authenticators,ads-interceptorId=authenticationInterceptor,ou=interceptors,ads-directoryServiceId=default,ou=config
> ou: authenticators
>
>
Re: svn commit: r1050378 - /directory/apacheds/trunk/server-config/src/main/resources/config.ldif
Posted by Emmanuel Lecharny <el...@gmail.com>.
On 12/17/10 4:00 PM, Pierre-Arnaud Marcelot wrote:
> Hi Antoine,
>
> I only added it for consistency with other entries in the configuration and for the following reason, mention by Emmanuel in the previous thread "Adding annotations to Configuration beans":
>
> Begin forwarded message:
>> On 12/6/10 6:00 PM, Pierre-Arnaud Marcelot wrote:
>>> On 6 déc. 2010, at 17:32, Alex Karasulu wrote:
>>>> Also as Kiran pointed out in his response, what if the contained elements are kept in a single attribute like this ads-compositeElement ?
>>> AFAIR, the 'ads-compositeElement' was introduced as kind of "quick hack" to ease the work on the reader class. I really think we can get rid of it easily with the annotation system.
>> ads-compositeElement has been created at the origin as a way to tell the reader that the element is a composite element. The reader was supposed to be completely generic, ie the java Beans could have been generated automatically (except that because it requires the development of a maven plugin, something I didn't want to do).
>>
>> Now that you have defined some @, sure this is redondant, but if someone decide to define a reader/writer in another language, then this element is probably necessary.
>
> That said, I 100% agree on the fact that this attribute is redundant and somehow unnecessary.
>
> I still think it should be removed for an easier (hand) writing of the configuration.
> Also, looks like the ConfigReader does not need it in its current implementation, as all objects were perfectly instantiated before my fix on the file.
>
> Maybe it's time to take a community-wide decision about these types of attributes (attributes extending 'ads-compositeElement').
>
> Regards,
> Pierre-Arnaud
>
> On 17 déc. 2010, at 15:31, Antoine Levy-Lambert wrote:
>
>> Hello Pierre-Alain,
>>
>> is this change needed ?
>>
>> I would think that these attributes are only needed when you are dealing with for instance a multi-valued string.
>>
>> But this is a list contained in a subentry ou=authenticators
>>
>> So I think that this ads-authenticators attribute is not required.
So let's remove it.
No need to add stack over stack without cleaning what is obsolete.
Nobody will be interested in the future to dig the code in order to
ressucitate the different layers, as if ADS was the city of Troy...
--
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com
Re: svn commit: r1050378 - /directory/apacheds/trunk/server-config/src/main/resources/config.ldif
Posted by Antoine Levy-Lambert <an...@gmx.de>.
Hello Pierre-Arnaud,
On 12/17/2010 10:00 AM, Pierre-Arnaud Marcelot wrote:
> Hi Antoine,
>
> I only added it for consistency with other entries in the configuration and for the following reason, mention by Emmanuel in the previous thread "Adding annotations to Configuration beans":
>
> Begin forwarded message:
>> On 12/6/10 6:00 PM, Pierre-Arnaud Marcelot wrote:
>>> On 6 déc. 2010, at 17:32, Alex Karasulu wrote:
>>>> Also as Kiran pointed out in his response, what if the contained elements are kept in a single attribute like this ads-compositeElement ?
>>> AFAIR, the 'ads-compositeElement' was introduced as kind of "quick hack" to ease the work on the reader class. I really think we can get rid of it easily with the annotation system.
>> ads-compositeElement has been created at the origin as a way to tell the reader that the element is a composite element. The reader was supposed to be completely generic, ie the java Beans could have been generated automatically (except that because it requires the development of a maven plugin, something I didn't want to do).
>>
>> Now that you have defined some @, sure this is redondant, but if someone decide to define a reader/writer in another language, then this element is probably necessary.
>
> That said, I 100% agree on the fact that this attribute is redundant and somehow unnecessary.
>
> I still think it should be removed for an easier (hand) writing of the configuration.
> Also, looks like the ConfigReader does not need it in its current implementation, as all objects were perfectly instantiated before my fix on the file.
>
> Maybe it's time to take a community-wide decision about these types of attributes (attributes extending 'ads-compositeElement').
>
There are two sorts of ads-compositeelements.
Some are containers containing entries. Others are multi-valued attributes.
I would see three possibilities
1) say that all container entries which are children of configuration
elements imply automatically that they contain objects to be added to a
list present in the containing element.
For example finding a subordinate entry ou=flowers the
ConfigPartitionReader would expect automatically to find flower entries
readable with a configuration bean of the ads-type of the subordinate
entries under ou=flowers.
The "container" field in the ConfigurationElement annotation would not
be required, we would just swallow automatically whatever containers are
lying below each configuration entry, recursively
the nice thing about this option is that it makes the least possible
metadata to manage
2) create a new multi valued attribute type ads-container, and use the
ads-container to list the subordinate entries to be read by the config
partition reader. For instance ads-container could have two values
flowers and vegetables, meaning that the config partition reader has to
search explicitly for ou=flowers and ou=vegetables below the current
entry. The nice thing is that it disentangles completely the case of
attribute based composite elements compared to entry based composite
elements
3) statu-quo. Disadvantage : unneeded redundant information is just
confusing
-
> Regards,
> Pierre-Arnaud
>
> On 17 déc. 2010, at 15:31, Antoine Levy-Lambert wrote:
>
Antoine
Re: svn commit: r1050378 - /directory/apacheds/trunk/server-config/src/main/resources/config.ldif
Posted by Pierre-Arnaud Marcelot <pa...@marcelot.net>.
Hi Antoine,
I only added it for consistency with other entries in the configuration and for the following reason, mention by Emmanuel in the previous thread "Adding annotations to Configuration beans":
Begin forwarded message:
> On 12/6/10 6:00 PM, Pierre-Arnaud Marcelot wrote:
>> On 6 déc. 2010, at 17:32, Alex Karasulu wrote:
>>> Also as Kiran pointed out in his response, what if the contained elements are kept in a single attribute like this ads-compositeElement ?
>> AFAIR, the 'ads-compositeElement' was introduced as kind of "quick hack" to ease the work on the reader class. I really think we can get rid of it easily with the annotation system.
> ads-compositeElement has been created at the origin as a way to tell the reader that the element is a composite element. The reader was supposed to be completely generic, ie the java Beans could have been generated automatically (except that because it requires the development of a maven plugin, something I didn't want to do).
>
> Now that you have defined some @, sure this is redondant, but if someone decide to define a reader/writer in another language, then this element is probably necessary.
That said, I 100% agree on the fact that this attribute is redundant and somehow unnecessary.
I still think it should be removed for an easier (hand) writing of the configuration.
Also, looks like the ConfigReader does not need it in its current implementation, as all objects were perfectly instantiated before my fix on the file.
Maybe it's time to take a community-wide decision about these types of attributes (attributes extending 'ads-compositeElement').
Regards,
Pierre-Arnaud
On 17 déc. 2010, at 15:31, Antoine Levy-Lambert wrote:
> Hello Pierre-Alain,
>
> is this change needed ?
>
> I would think that these attributes are only needed when you are dealing with for instance a multi-valued string.
>
> But this is a list contained in a subentry ou=authenticators
>
> So I think that this ads-authenticators attribute is not required.
>
> And it is redundant with the other items below.
>
> Regards,
> Antoine
>
> On 12/17/2010 8:32 AM, pamarcelot@apache.org wrote:
>> Author: pamarcelot
>> Date: Fri Dec 17 13:32:30 2010
>> New Revision: 1050378
>>
>> URL: http://svn.apache.org/viewvc?rev=1050378&view=rev
>> Log:
>> Added missing attribute 'ads-authenticators'.
>>
>> Modified:
>> directory/apacheds/trunk/server-config/src/main/resources/config.ldif
>>
>> Modified: directory/apacheds/trunk/server-config/src/main/resources/config.ldif
>> URL: http://svn.apache.org/viewvc/directory/apacheds/trunk/server-config/src/main/resources/config.ldif?rev=1050378&r1=1050377&r2=1050378&view=diff
>> ==============================================================================
>> --- directory/apacheds/trunk/server-config/src/main/resources/config.ldif (original)
>> +++ directory/apacheds/trunk/server-config/src/main/resources/config.ldif Fri Dec 17 13:32:30 2010
>> @@ -78,6 +78,9 @@ ads-interceptororder: 2
>> ads-interceptorclassname: org.apache.directory.server.core.authn.AuthenticationInterceptor
>> ads-interceptorid: authenticationInterceptor
>> ads-enabled: TRUE
>> +ads-authenticators: anonymousauthenticator
>> +ads-authenticators: simpleauthenticator
>> +ads-authenticators: strongauthenticator
>>
>> dn: ou=authenticators,ads-interceptorId=authenticationInterceptor,ou=interceptors,ads-directoryServiceId=default,ou=config
>> ou: authenticators
>>
>>
>