On Tue, Nov 27, 2012 at 7:31 PM, Pierre-Arnaud Marcelot <[email protected]>wrote:
> On 27 nov. 2012, at 14:46, Kiran Ayyagari <[email protected]> wrote: > > On Tue, Nov 27, 2012 at 7:04 PM, Pierre-Arnaud Marcelot > <[email protected]>wrote: > >> Hi Kiran, >> >> This is not exactly what we usually call a setter method as we never >> clear the list of attributes and we keep adding things to it each time we >> use the method. >> >> After the SyncReplConfiguration object is instanciated, there's no way to >> set the actual list of attributes to something you want or to reset it. >> >> actually this isn't supposed to be modified after instantiating, but I > have never added such protection in this class > > > Hehe, you never know what people will do... ;-) > > yeap, lesson learned > That's why I changed it a few days ago to be able to use in the >> configuration editor where I need to be able to store values from the user. >> >> just curious, aren't we supposed to use the ReplConsumerBean for > configuration editing? > > > Yeah, you're right, that's the class we're using. > I might have been using SyncReplConfiguration at first and figured out the > setAttributes() method cumulative behavior because of it, I guess. > It's not used anymore in the editor now. > > great > I'm not against a cumulative method, but I think it would better to have >> it named addAttributes() and leave the setter as a way to really set the >> actual list of attributes. >> >> WDYT? >> >> no, this is fine, I have thought of moving this(addition of + to > attribute list) to the server side but didn't, apparently now > is the time to do it > > > Yeah, that might be safer, especially after we have a graphical editor > where people can do lots of nasty stuff. > It '+' is mandatory for the replication logic on the server-side, I think > too the server should add it. > Otherwise, the replication can't work properly. > > One last thing. > In the current implementation we test the presence of the lower-cased attr > in the list but we're storing the attribute as-is. > So I guess, it might be possible to add the same attribute multiple times > ('ObjectClass', 'objectClass', 'objectclass' for example). > > yeah, this logic is no more present, (otoh, this is a mistake but adding the same attribute multiple times is harmless cause server works with AttributeType instances) Regards, > Pierre-Arnaud > > > Regards, >> Pierre-Arnaud >> >> On 27 nov. 2012, at 14:16, [email protected] wrote: >> >> > Author: kayyagari >> > Date: Tue Nov 27 13:16:43 2012 >> > New Revision: 1414171 >> > >> > URL: http://svn.apache.org/viewvc?rev=1414171&view=rev >> > Log: >> > do not remove operational attributes from the requested attribute types >> > >> > Modified: >> > >> >> directory/apacheds/trunk/protocol-ldap/src/main/java/org/apache/directory/server/ldap/replication/SyncReplConfiguration.java >> > >> > Modified: >> directory/apacheds/trunk/protocol-ldap/src/main/java/org/apache/directory/server/ldap/replication/SyncReplConfiguration.java >> > URL: >> http://svn.apache.org/viewvc/directory/apacheds/trunk/protocol-ldap/src/main/java/org/apache/directory/server/ldap/replication/SyncReplConfiguration.java?rev=1414171&r1=1414170&r2=1414171&view=diff >> > >> ============================================================================== >> > --- >> directory/apacheds/trunk/protocol-ldap/src/main/java/org/apache/directory/server/ldap/replication/SyncReplConfiguration.java >> (original) >> > +++ >> directory/apacheds/trunk/protocol-ldap/src/main/java/org/apache/directory/server/ldap/replication/SyncReplConfiguration.java >> Tue Nov 27 13:16:43 2012 >> > @@ -304,11 +304,26 @@ public class SyncReplConfiguration imple >> > */ >> > public void setAttributes( String[] attrs ) >> > { >> > - attributes.clear(); >> > - >> > - for ( String attr : attrs ) >> > + if ( attrs == null ) >> > { >> > - attributes.add( attr ); >> > + throw new IllegalArgumentException( "attributes to be >> replicated cannot be null or empty" ); >> > + } >> > + >> > + // if user specified some attributes then remove the * from >> attributes >> > + // NOTE: if the user specifies * in the given array that >> eventually gets added later >> > + if ( attrs.length > 0 ) >> > + { >> > + attributes.remove( SchemaConstants.ALL_USER_ATTRIBUTES ); >> > + } >> > + >> > + for ( String at : attrs ) >> > + { >> > + at = at.trim(); >> > + >> > + if ( !attributes.contains( Strings.toLowerCase( at ) ) ) >> > + { >> > + attributes.add( at ); >> > + } >> > } >> > } >> > >> > >> > >> >> > > > -- > Kiran Ayyagari > http://keydap.com > > > -- Kiran Ayyagari http://keydap.com
