Directory ApacheDS
  1. Directory ApacheDS
  2. DIRSERVER-1300

Only adding from LDIF is possible with injectEntries() in IntegrationUtils

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.5.4
    • Fix Version/s: 1.5.5
    • Component/s: core-integ
    • Labels:
      None

      Description

      The method org.apache.directory.server.core.integ.IntegrationUtils.injectEntries(DirectoryService, String) only supports adding entries - it assumes that there are no changetype: something-other-than-add entries in the LDIF. This greatly complicates modifying the intergration testing server's schema.

      So the following LDIF cannot be currently processed by injectEntries:

      version: 1
      dn: cn=schema
      changetype: modify
      add: attributeTypes
      attributeTypes: ( 1.3.6.1.4.1.12345.1.1 NAME 'customAttr1'
      DESC 'custom attribute 1'
      EQUALITY caseIgnoreMatch
      SYNTAX 1.3.6.1.4.1.1466.115.121.1.15
      SINGLE-VALUE )
      attributeTypes: ( 1.3.6.1.4.1.12345.1.2 NAME 'customAttr2'
      DESC 'custom attribute 2'
      EQUALITY caseIgnoreMatch
      SYNTAX 1.3.6.1.4.1.1466.115.121.1.15
      SINGLE-VALUE )
      -
      add: objectClasses
      objectClasses: ( .3.6.1.4.1.12345.2.1
      NAME 'customClass1'
      SUP top
      STRUCTURAL
      MUST ( cn $ customAttr1 )
      MAY ( customAttr2 ) )

      I've tracked down the problem and found out it's quite simple to add support for the remaining change types:

      Index: src/main/java/org/apache/directory/server/core/integ/IntegrationUtils.java
      ===================================================================
      — src/main/java/org/apache/directory/server/core/integ/IntegrationUtils.java (revision 731909)
      +++ src/main/java/org/apache/directory/server/core/integ/IntegrationUtils.java (working copy)
      @@ -99,8 +99,19 @@

      for ( LdifEntry entry : entries )
      {

      • service.getAdminSession().add(
      • new DefaultServerEntry( service.getRegistries(), entry.getEntry() ) );
        + if ( entry.isChangeAdd() )
        + { + service.getAdminSession().add( new DefaultServerEntry( service.getRegistries(), entry.getEntry() ) ); + + }

        + else if ( entry.isChangeModify() )
        +

        { + service.getAdminSession().modify( entry.getDn(), entry.getModificationItems() ); + }

        + else
        +

        { + throw new NamingException( "Unsupported changetype found in LDIF: " + entry.getChangeType() ); + }

        }
        }

      I'll attach the patch in a minute.

        Activity

        Hide
        Aleksander Adamowski PZU added a comment -

        The patch that adds support for changetype modify.

        Show
        Aleksander Adamowski PZU added a comment - The patch that adds support for changetype modify.
        Hide
        Aleksander Adamowski PZU added a comment -
        Show
        Aleksander Adamowski PZU added a comment - You've marked this as fixed for 1.5.5, however I don't see any changes on the trunk: http://svn.apache.org/repos/asf/directory/apacheds/trunk/core-integ/src/main/java/org/apache/directory/server/core/integ/IntegrationUtils.java
        Hide
        Emmanuel Lecharny added a comment -

        1.5.5 is not out yet

        I just edited the issue and set the fix for version 1.5.5. It's still open (check the Status).

        Show
        Emmanuel Lecharny added a comment - 1.5.5 is not out yet I just edited the issue and set the fix for version 1.5.5. It's still open (check the Status).
        Hide
        Emmanuel Lecharny added a comment -
        Show
        Emmanuel Lecharny added a comment - Applied Aleksander's patch : http://svn.apache.org/viewvc?rev=738811&view=rev
        Hide
        Aleksander Adamowski PZU added a comment -

        I think the patch didn't apply correctly because the old unconditional add is still there

        for ( LdifEntry entry : entries )
        {
        HERE--> service.getAdminSession().add(
        new DefaultServerEntry( service.getRegistries(), entry.getEntry() ) );
        if ( entry.isChangeAdd() )

        { service.getAdminSession().add( new DefaultServerEntry( service.getRegistries(), entry.getEntry() ) ); }

        else if ( entry.isChangeModify() )

        { service.getAdminSession().modify( entry.getDn(), entry.getModificationItems() ); }

        else

        { String message = "Unsupported changetype found in LDIF: " + entry.getChangeType(); LOG.error( message ); throw new NamingException( message ); }

        }

        I think it will throw the Exception before getting to the conditional check. These two lines should be deleted, there's already an add() invocation after entry.isChangeAdd() gets checked. If you try to add() unconditionally, you'll get an exception if the change isn't actually an add.

        Show
        Aleksander Adamowski PZU added a comment - I think the patch didn't apply correctly because the old unconditional add is still there for ( LdifEntry entry : entries ) { HERE--> service.getAdminSession().add( new DefaultServerEntry( service.getRegistries(), entry.getEntry() ) ); if ( entry.isChangeAdd() ) { service.getAdminSession().add( new DefaultServerEntry( service.getRegistries(), entry.getEntry() ) ); } else if ( entry.isChangeModify() ) { service.getAdminSession().modify( entry.getDn(), entry.getModificationItems() ); } else { String message = "Unsupported changetype found in LDIF: " + entry.getChangeType(); LOG.error( message ); throw new NamingException( message ); } } I think it will throw the Exception before getting to the conditional check. These two lines should be deleted, there's already an add() invocation after entry.isChangeAdd() gets checked. If you try to add() unconditionally, you'll get an exception if the change isn't actually an add.
        Hide
        Emmanuel Lecharny added a comment -

        what a dumb a** I'm !

        Let me fix that...

        Show
        Emmanuel Lecharny added a comment - what a dumb a** I'm ! Let me fix that...
        Show
        Emmanuel Lecharny added a comment - Fixed in : http://svn.apache.org/viewvc?rev=738820&view=rev
        Hide
        Emmanuel Lecharny added a comment -

        closed

        Show
        Emmanuel Lecharny added a comment - closed
        Hide
        Aleksander Adamowski PZU added a comment -

        Confirming that the fix works with the latest 1.5.5-SNAPSHOT.

        Show
        Aleksander Adamowski PZU added a comment - Confirming that the fix works with the latest 1.5.5-SNAPSHOT.
        Hide
        Steve Jones added a comment -

        I think the same issue is present in AbstractState.injectLdifs (which is used by the @ApplyLdifFiles annotation).

        Show
        Steve Jones added a comment - I think the same issue is present in AbstractState.injectLdifs (which is used by the @ApplyLdifFiles annotation).

          People

          • Assignee:
            Unassigned
            Reporter:
            Aleksander Adamowski PZU
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development