Open Darkheir opened 13 years ago
ldapEnv seems to be dependent on the LDAP server rather than on groups or persons. Therefore they should be set in the constructor of the DAO.
The parameters of a DAO is usually a single generic object.
We could add the type and the id in the attributes object, this way the parameters would be a single object and we would have:
public ldapDao (String ldap_server, String ldap_account, String ldap_password) { // we set the ldapEnv } public void create (Attributes attributes) { }
public Attributes retrieve (Attributes attributes) { }
public void update (Attributes attributes) { }
public void delete (Attributes attributes) { }
It looks quite better. You (and maybe a colleague with you) should try to refactor the application like that.
I'm having some trouble realising the refactoring:
I can see the commons parts between "ldapuser" and "ldapgroup" but not with the methods acting on the users inside a group (adding a user in a specific group, removing him, ...) There is a lot of instructions changing from one method to an other and since the CRUD class has to be generic i don't really know how to implement it. Do I have to put the coding lines differing in the class calling the CRUD, or do i Have to create an intermediate class doing the instructions and calling the CRUD? An other alternative would be to create a class extending the CRUD where we'll be able to define some new methods.
I really don't know what is the best alternative, so it'll be nice if you could help me.
This is exactly when the DAO pattern becomes interesting. The idea is really to split a process into one part that depends on the repository type, and the other part that depends on the object type to manage.
Here are some pseudo-code :
abstract class C {
abstract serialize();
}
class C1 extends C {
@Override serialize() {
...
}
}
class C2 extends C {
@Override serialize() {
...
}
}
class Repository {
write(C c) {
writeSerialization(c.serialize());
}
...
}
1) We finished the conception of all of the new classes. The classes LDap , LDapUser and LDAPGroup are delete.
=>In the data manipulation layer: The new classes are User, Group which extends The abstract class Entry. -The class Entry only have the method (toLDAPAttributes) -The class User has the attributes (email,password,login,firstname,lastname)+getters+setters -The class Group has the attributes(owner, groupName, members)+getters+setters+addMember+deleteMember+displayMembers
=> We have the class LDAPDirectory manages the connection with the data source to obtain and store data. -attributes:ldap_account,ldapEnv, ldapContext -methodes: create(Entry entry), Retrieve(Sting id), update(Entry entry), delete(Entry entry)
2) Next step: We are checking in the controllers classes all the methods that require LDAP and we are making the chages needed.
3) We will verify by testing Dolomite if the compilation of the new code is possible without any exception.
4) We will do a new LDAPDirectory unit test to be sure everything is well.
The aim is to have a general Data Access Object (DAO) for LDAP.
My proposition is to use a CRUD architecture with the same functions for user, group and member group manipulations.
public void create (String type, Hashtable<String, String> ldapEnv, Attributes attributes) { }
public Attributes retrieve (String type, Hashtable ldapEnv, String id) { }
public void update (String type, Hashtable ldapEnv, String id, String attributeName, String value,) { }
public void delete (String type, Hashtable ldapEnv, String id, ArrayList members) { }
type would be equal to "user", "group" or "memberGroup" to make the difference between an action on a user, or a group. Thoses methods are quite general and are taking advantages of the similarities between the different actions to refactor the code.
The differents informations mandatory to add a user, a group or a group member will be grouped in an Attributes object.
For the Delete method, the "ArrayList members" would be "null" for a user or a group since we don't need this to delete them. (I'm not sure if this is possible in Java, an other solution would be to create 2 methods Delete, one with one more parameters than the other : "ArrayList members")
This structure would change the definition of the object attributes which would have to be done in the LdapUser and LdapGroup classes instead of Ldap.