Author: elecharny Date: Fri Jan 5 06:12:40 2007 New Revision: 493037 URL: http://svn.apache.org/viewvc?view=rev&rev=493037 Log: Fix for DIRSERVER-791. The MODIFY operation didn't handled correctly values modification : the attribute itself was completly removed even if just a value was to be removed. Fixed another problem : when comparing the RDN with the attribute, if the attribyteType was not an OID, then there was a difference even if oid(RDN) = attributeType OID. A new parameter (OIDRegistry) was added to all the method to handle that case Also fixed the testcase Modified: directory/branches/apacheds/1.0/core/src/main/java/org/apache/directory/server/core/schema/SchemaChecker.java directory/branches/apacheds/1.0/core/src/main/java/org/apache/directory/server/core/schema/SchemaService.java directory/branches/apacheds/1.0/core/src/test/java/org/apache/directory/server/core/schema/SchemaCheckerTest.java Modified: directory/branches/apacheds/1.0/core/src/main/java/org/apache/directory/server/core/schema/SchemaChecker.java URL: http://svn.apache.org/viewvc/directory/branches/apacheds/1.0/core/src/main/java/org/apache/directory/server/core/schema/SchemaChecker.java?view=diff&rev=493037&r1=493036&r2=493037 ============================================================================== --- directory/branches/apacheds/1.0/core/src/main/java/org/apache/directory/server/core/schema/SchemaChecker.java (original) +++ directory/branches/apacheds/1.0/core/src/main/java/org/apache/directory/server/core/schema/SchemaChecker.java Fri Jan 5 06:12:40 2007 @@ -387,7 +387,7 @@ // from here on the modify operation replaces specific values // of the Rdn attribute so we must check to make sure all the old // rdn attribute values are present in the replacement set - String rdnValue = getRdnValue( id, name ); + String rdnValue = getRdnValue( id, name, oidRegistry ); for ( int ii = 0; ii < attribute.size(); ii++ ) { // if the old rdn value is not in the rdn attribute then @@ -426,7 +426,7 @@ * @param attributes the attributes being modified * @throws NamingException if the modify operation is removing an Rdn attribute */ - public static void preventRdnChangeOnModifyReplace( Name name, int mod, Attributes attributes ) + public static void preventRdnChangeOnModifyReplace( Name name, int mod, Attributes attributes, OidRegistry oidRegistry ) throws NamingException { if ( mod != DirContext.REPLACE_ATTRIBUTE ) @@ -460,7 +460,7 @@ // from here on the modify operation replaces specific values // of the Rdn attribute so we must check to make sure all the old // rdn attribute values are present in the replacement set - String rdnValue = getRdnValue( id, name ); + String rdnValue = getRdnValue( id, name, oidRegistry ); Attribute rdnAttr = attributes.get( id ); for ( int ii = 0; ii < rdnAttr.size(); ii++ ) { @@ -536,7 +536,7 @@ // from here on the modify operation only deletes specific values // of the Rdn attribute so we must check if one of those values // are used by the Rdn attribute value pair for the name of the entry - String rdnValue = getRdnValue( id, name ); + String rdnValue = getRdnValue( id, name, oidRegistry ); for ( int ii = 0; ii < attribute.size(); ii++ ) { if ( rdnValue.equals( attribute.get( ii ) ) ) @@ -573,7 +573,7 @@ * @param attributes the attributes being modified * @throws NamingException if the modify operation is removing an Rdn attribute */ - public static void preventRdnChangeOnModifyRemove( Name name, int mod, Attributes attributes ) + public static void preventRdnChangeOnModifyRemove( Name name, int mod, Attributes attributes, OidRegistry oidRegistry ) throws NamingException { if ( mod != DirContext.REMOVE_ATTRIBUTE ) @@ -607,7 +607,7 @@ // from here on the modify operation only deletes specific values // of the Rdn attribute so we must check if one of those values // are used by the Rdn attribute value pair for the name of the entry - String rdnValue = getRdnValue( id, name ); + String rdnValue = getRdnValue( id, name, oidRegistry ); Attribute rdnAttr = attributes.get( id ); for ( int ii = 0; ii < rdnAttr.size(); ii++ ) { @@ -634,19 +634,38 @@ * * @param id the attribute id of the Rdn attribute to return * @param name the distinguished name of the entry + * @param oidRegistry the OID registry * @return the Rdn attribute value corresponding to the id, or null if the * attribute is not an rdn attribute * @throws NamingException if the name is malformed in any way */ - private static String getRdnValue( String id, Name name ) throws NamingException + private static String getRdnValue( String id, Name name, OidRegistry oidRegistry ) throws NamingException { + // Transform the rdnAttrId to it's OID counterPart + String idOid = oidRegistry.getOid( id ); + + if ( idOid == null ) + { + log.error( "The id {} does not have any OID. It should be a wrong AttributeType.", id); + throw new NamingException( "Wrong AttributeType, does not have an associated OID : " + id ); + } + String[] comps = NamespaceTools.getCompositeComponents( name.get( name.size() - 1 ) ); for ( int ii = 0; ii < comps.length; ii++ ) { String rdnAttrId = NamespaceTools.getRdnAttribute( comps[ii] ); + + // Transform the rdnAttrId to it's OID counterPart + String rdnAttrOid = oidRegistry.getOid( rdnAttrId ); + + if ( rdnAttrOid == null ) + { + log.error( "The id {} does not have any OID. It should be a wrong AttributeType.", rdnAttrOid); + throw new NamingException( "Wrong AttributeType, does not have an associated OID : " + rdnAttrOid ); + } - if ( rdnAttrId.equalsIgnoreCase( id ) ) + if ( rdnAttrOid.equalsIgnoreCase( idOid ) ) { return NamespaceTools.getRdnValue( comps[ii] ); } Modified: directory/branches/apacheds/1.0/core/src/main/java/org/apache/directory/server/core/schema/SchemaService.java URL: http://svn.apache.org/viewvc/directory/branches/apacheds/1.0/core/src/main/java/org/apache/directory/server/core/schema/SchemaService.java?view=diff&rev=493037&r1=493036&r2=493037 ============================================================================== --- directory/branches/apacheds/1.0/core/src/main/java/org/apache/directory/server/core/schema/SchemaService.java (original) +++ directory/branches/apacheds/1.0/core/src/main/java/org/apache/directory/server/core/schema/SchemaService.java Fri Jan 5 06:12:40 2007 @@ -709,13 +709,13 @@ if ( modOp == DirContext.REMOVE_ATTRIBUTE ) { - SchemaChecker.preventRdnChangeOnModifyRemove( name, modOp, mods ); + SchemaChecker.preventRdnChangeOnModifyRemove( name, modOp, mods, this.globalRegistries.getOidRegistry() ); SchemaChecker.preventStructuralClassRemovalOnModifyRemove( ocRegistry, name, modOp, mods, objectClass ); } if ( modOp == DirContext.REPLACE_ATTRIBUTE ) { - SchemaChecker.preventRdnChangeOnModifyReplace( name, modOp, mods ); + SchemaChecker.preventRdnChangeOnModifyReplace( name, modOp, mods, this.globalRegistries.getOidRegistry() ); SchemaChecker.preventStructuralClassRemovalOnModifyReplace( ocRegistry, name, modOp, mods ); assertNumberOfAttributeValuesValid( mods ); } @@ -909,15 +909,49 @@ throw new LdapNoSuchAttributeException(); } - // for required attributes we need to check if all values are removed - // if so then we have a schema violation that must be thrown - if ( isRequired( change.getID(), objectClass ) && isCompleteRemoval( change, entry ) ) + // We may have to remove the attribute or only some values + if ( change.size() == 0 ) { - log.error( "Trying to remove a required attribute: " + change.getID() ); - throw new LdapSchemaViolationException( ResultCodeEnum.OBJECTCLASSVIOLATION ); + // No value : we have to remove the entire attribute + // Check that we aren't removing a MUST attribute + if ( isRequired( change.getID(), objectClass ) ) + { + log.error( "Trying to remove a required attribute: " + change.getID() ); + throw new LdapSchemaViolationException( ResultCodeEnum.OBJECTCLASSVIOLATION ); + } + } + else + { + // for required attributes we need to check if all values are removed + // if so then we have a schema violation that must be thrown + if ( isRequired( change.getID(), objectClass ) && isCompleteRemoval( change, entry ) ) + { + log.error( "Trying to remove a required attribute: " + change.getID() ); + throw new LdapSchemaViolationException( ResultCodeEnum.OBJECTCLASSVIOLATION ); + } + + // Now remove the attribute and all its values + Attribute modified = tmpEntry.remove( change.getID() ); + + // And inject back the values except the ones to remove + NamingEnumeration values = change.getAll(); + + while ( values.hasMoreElements() ) + { + modified.remove( values.next() ); + } + + // ok, done. Last check : if the attribute does not content any more value; + // and if it's a MUST one, we should thow an exception + if ( ( modified.size() == 0 ) && isRequired( change.getID(), objectClass ) ) + { + log.error( "Trying to remove a required attribute: " + change.getID() ); + throw new LdapSchemaViolationException( ResultCodeEnum.OBJECTCLASSVIOLATION ); + } + + // Put back the attribute in the entry + tmpEntry.put( modified ); } - - tmpEntry.remove( change.getID() ); SchemaChecker.preventRdnChangeOnModifyRemove( name, modOp, change, this.globalRegistries.getOidRegistry() ); Modified: directory/branches/apacheds/1.0/core/src/test/java/org/apache/directory/server/core/schema/SchemaCheckerTest.java URL: http://svn.apache.org/viewvc/directory/branches/apacheds/1.0/core/src/test/java/org/apache/directory/server/core/schema/SchemaCheckerTest.java?view=diff&rev=493037&r1=493036&r2=493037 ============================================================================== --- directory/branches/apacheds/1.0/core/src/test/java/org/apache/directory/server/core/schema/SchemaCheckerTest.java (original) +++ directory/branches/apacheds/1.0/core/src/test/java/org/apache/directory/server/core/schema/SchemaCheckerTest.java Fri Jan 5 06:12:40 2007 @@ -243,13 +243,13 @@ attributes.put( "cn", "does not matter" ); // postive test which should pass - SchemaChecker.preventRdnChangeOnModifyRemove( name, mod, attributes ); + SchemaChecker.preventRdnChangeOnModifyRemove( name, mod, attributes, registries.getOidRegistry() ); // test should fail since we are removing the ou attribute attributes.put( new BasicAttribute( "ou" ) ); try { - SchemaChecker.preventRdnChangeOnModifyRemove( name, mod, attributes ); + SchemaChecker.preventRdnChangeOnModifyRemove( name, mod, attributes, registries.getOidRegistry() ); fail( "should never get here due to a LdapSchemaViolationException being thrown" ); } catch ( LdapSchemaViolationException e ) @@ -261,13 +261,13 @@ name = new LdapDN( "ou=users+cn=system users,dc=example,dc=com" ); attributes = new BasicAttributes( true ); attributes.put( "sn", "does not matter" ); - SchemaChecker.preventRdnChangeOnModifyRemove( name, mod, attributes ); + SchemaChecker.preventRdnChangeOnModifyRemove( name, mod, attributes, registries.getOidRegistry() ); // test for failure when modifying Rdn attribute in multi attribute Rdn attributes.put( new BasicAttribute( "cn" ) ); try { - SchemaChecker.preventRdnChangeOnModifyRemove( name, mod, attributes ); + SchemaChecker.preventRdnChangeOnModifyRemove( name, mod, attributes, registries.getOidRegistry() ); fail( "should never get here due to a LdapSchemaViolationException being thrown" ); } catch ( LdapSchemaViolationException e ) @@ -279,14 +279,14 @@ // is not used when composing the Rdn attributes = new BasicAttributes( true ); attributes.put( "ou", "container" ); - SchemaChecker.preventRdnChangeOnModifyRemove( name, mod, attributes ); + SchemaChecker.preventRdnChangeOnModifyRemove( name, mod, attributes, registries.getOidRegistry() ); // now let's make it fail again just by providing the right value for ou (users) attributes = new BasicAttributes( true ); attributes.put( "ou", "users" ); try { - SchemaChecker.preventRdnChangeOnModifyRemove( name, mod, attributes ); + SchemaChecker.preventRdnChangeOnModifyRemove( name, mod, attributes, registries.getOidRegistry() ); fail( "should never get here due to a LdapSchemaViolationException being thrown" ); } catch ( LdapSchemaViolationException e ) @@ -308,13 +308,13 @@ attributes.put( "cn", "does not matter" ); // postive test which should pass - SchemaChecker.preventRdnChangeOnModifyReplace( name, mod, attributes ); + SchemaChecker.preventRdnChangeOnModifyReplace( name, mod, attributes, registries.getOidRegistry() ); // test should fail since we are removing the ou attribute attributes.put( new BasicAttribute( "ou" ) ); try { - SchemaChecker.preventRdnChangeOnModifyReplace( name, mod, attributes ); + SchemaChecker.preventRdnChangeOnModifyReplace( name, mod, attributes, registries.getOidRegistry() ); fail( "should never get here due to a LdapSchemaViolationException being thrown" ); } catch ( LdapSchemaViolationException e ) @@ -326,13 +326,13 @@ name = new LdapDN( "ou=users+cn=system users,dc=example,dc=com" ); attributes = new BasicAttributes( true ); attributes.put( "sn", "does not matter" ); - SchemaChecker.preventRdnChangeOnModifyReplace( name, mod, attributes ); + SchemaChecker.preventRdnChangeOnModifyReplace( name, mod, attributes, registries.getOidRegistry() ); // test for failure when modifying Rdn attribute in multi attribute Rdn attributes.put( new BasicAttribute( "cn" ) ); try { - SchemaChecker.preventRdnChangeOnModifyReplace( name, mod, attributes ); + SchemaChecker.preventRdnChangeOnModifyReplace( name, mod, attributes, registries.getOidRegistry() ); fail( "should never get here due to a LdapSchemaViolationException being thrown" ); } catch ( LdapSchemaViolationException e ) @@ -345,14 +345,14 @@ attributes = new BasicAttributes( true ); attributes.put( "ou", "container" ); attributes.put( "ou", "users" ); - SchemaChecker.preventRdnChangeOnModifyReplace( name, mod, attributes ); + SchemaChecker.preventRdnChangeOnModifyReplace( name, mod, attributes, registries.getOidRegistry() ); // now let's make it fail by not including the old value for ou (users) attributes = new BasicAttributes( true ); attributes.put( "ou", "container" ); try { - SchemaChecker.preventRdnChangeOnModifyReplace( name, mod, attributes ); + SchemaChecker.preventRdnChangeOnModifyReplace( name, mod, attributes, registries.getOidRegistry() ); fail( "should never get here due to a LdapSchemaViolationException being thrown" ); } catch ( LdapSchemaViolationException e )