[389-devel] lib389 fixes for enableReplication/Replica.add

[thierry bordaz]

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.fedoraproject.org/pipermail/389-devel/attachments/20131128/0e0c2815/attachment.html>
-------------- next part --------------
commit eb1576aa5fa059e7a59c82f55abdb0760e1f5699
Author: Thierry bordaz (tbordaz) <tbordaz at redhat.com>
Date:   Thu Nov 28 14:33:33 2013 +0100

    Change enableReplication/Replica.add interface to use role. + error handling with exception
    
    Reviewed by: ?

diff --git a/lib389/__init__.py b/lib389/__init__.py
index 42672b4..b69151f 100644
--- a/lib389/__init__.py
+++ b/lib389/__init__.py
@@ -1250,39 +1250,42 @@ class DirSrv(SimpleLDAPObject):
     def enableReplication(self, suffix=None, role=None, replicaId=CONSUMER_REPLICAID, binddn=None):
         if not suffix:
             log.fatal("enableReplication: suffix not specified")
-            return 1
+            raise ValueError("suffix missing")
         
         if not role:
             log.fatal("enableReplication: replica role not specify (REPLICAROLE_*)")
-            return 1
+            raise ValueError("role missing")
         
         # 
         # Check the validity of the parameters
         #
         
         # First role and replicaID
-        if role == REPLICAROLE_MASTER:
+        if role != REPLICAROLE_MASTER and role != REPLICAROLE_HUB and role != REPLICAROLE_CONSUMER:
+            log.fatal("enableReplication: replica role invalid (%s) " % role)
+            raise ValueError("invalid role: %s" % role)
+        
             # master
             replica_type = REPLICA_RDWR_TYPE
         else:
             # hub or consumer
             replica_type = REPLICA_RDONLY_TYPE
         
-        if replica_type == REPLICA_RDWR_TYPE:
+        if role == REPLICAROLE_MASTER:
             # check the replicaId [1..CONSUMER_REPLICAID[
             if not decimal.Decimal(replicaId) or (replicaId <= 0) or (replicaId >=CONSUMER_REPLICAID):
                 log.fatal("enableReplication: invalid replicaId (%s) for a RW replica" % replicaId)
-                return 1
+                raise ValueError("invalid replicaId %d (expected [1..CONSUMER_REPLICAID[" % replicaId)
         elif replicaId != CONSUMER_REPLICAID:
             # check the replicaId is CONSUMER_REPLICAID
             log.fatal("enableReplication: invalid replicaId (%s) for a Read replica (expected %d)" % (replicaId, CONSUMER_REPLICAID))
-            return 1
+            raise ValueError("invalid replicaId: %d for HUB/CONSUMER replicaId is CONSUMER_REPLICAID" % replicaId)
             
         # Now check we have a suffix
         entries_backend = self.getBackendsForSuffix(suffix, ['nsslapd-suffix'])
         if not entries_backend:
             log.fatal("enableReplication: enable to retrieve the backend for %s" % suffix)
-            return 1
+            raise ValueError("no backend for suffix %s" % suffix)
         
         ent = entries_backend[0]
         if normalizeDN(suffix) != normalizeDN(ent.getValue('nsslapd-suffix')):
@@ -1309,7 +1312,7 @@ class DirSrv(SimpleLDAPObject):
         self.replica.create_repl_manager()
         
         # then enable replication
-        ret = self.replica.add(suffix=suffix, binddn=binddn, rtype=replica_type, rid=replicaId)
+        ret = self.replica.add(suffix=suffix, binddn=binddn, role=role, rid=replicaId)
         
         return ret
 
diff --git a/lib389/brooker.py b/lib389/brooker.py
index 932d9fc..18d7edd 100644
--- a/lib389/brooker.py
+++ b/lib389/brooker.py
@@ -622,14 +622,14 @@ class Replica(object):
 
 
 
-    def add(self, suffix, binddn, bindpw=None, rtype=REPLICA_RDONLY_TYPE, rid=None, tombstone_purgedelay=None, purgedelay=None, referrals=None, legacy=False):
+    def add(self, suffix, binddn, bindpw=None, role=None, rid=None, tombstone_purgedelay=None, purgedelay=None, referrals=None, legacy=False):
         """Setup a replica entry on an existing suffix.
             @param suffix - dn of suffix
             @param binddn - the replication bind dn for this replica
                             can also be a list ["cn=r1,cn=config","cn=r2,cn=config"]
             @param bindpw - used to eventually provision the replication entry
 
-            @param rtype - REPLICA_RDWR_TYPE (master) or REPLICA_RDONLY_TYPE (hub/consumer)
+            @param role - REPLICAROLE_MASTER, REPLICAROLE_HUB or REPLICAROLE_CONSUMER
             @param rid - replica id or - if not given - an internal sequence number will be assigned
 
             # further args
@@ -646,12 +646,21 @@ class Replica(object):
              binddn
             TODO: this method does not update replica type
         """
-        # set default values
-        if rtype == MASTER_TYPE:
+        # Check validity of role
+        if not role:
+            self.log.fatal("Replica.create: replica role not specify (REPLICAROLE_*)")
+            raise ValueError("role missing")
+
+        if role != REPLICAROLE_MASTER and role != REPLICAROLE_HUB and role != REPLICAROLE_CONSUMER:
+            self.log.fatal("enableReplication: replica role invalid (%s) " % role)
+            raise ValueError("invalid role: %s" % role)
+        
+        # role is fine, set the replica type
+        if role == REPLICAROLE_MASTER:
             rtype = REPLICA_RDWR_TYPE
         else:
             rtype = REPLICA_RDONLY_TYPE
-
+            
         if legacy:
             legacy = 'on'
         else:
@@ -687,7 +696,7 @@ class Replica(object):
             'nsds5replicalegacyconsumer': legacy,
             'nsds5replicabinddn': binddnlist
         })
-        if rtype != LEAF_TYPE:
+        if role != REPLICAROLE_CONSUMER:
             entry.setValues('nsds5flags', "1")
 
         # other args