Is subclassing of generated models a bad idea?

[matejak]

Hello, I was thinking that maybe I could let the ORM generate a base class for me which I would then subclass, so the application logic could better integrate with the module.
However, I am starting to have doubts whether this is a good idea. Most notably, the following code doesn't work (lets' assume that we have a working conn object:

top_dn = "dc=example,dc=com"

BaseUser = ldap3_orm.EntryType("uid={uid},{base_dn}", ["inetOrgPerson"], schema=conn)

# already this doesn't work
class User(BaseUser):
    base_dn = "ou=people," + top_dn

class GreenUser(BaseUser):
    @property
    def base_dn(self):
        ret = "ou=green,ou=people," + top_dn
        return ret

The code crashes with a traceback, where the ldap-orm part

~/tests/ldap-orm/venv/lib/python3.9/site-packages/ldap3_orm/entry.py in __init__(self, dn, object_classes, schema, *args, **kwargs)
    377         self._dn = dn
    378         self._object_classes = object_classes
--> 379         self._objDef = ObjectDef(object_classes, schema, *args, **kwargs)
    380 
    381     def __call__(self, **kwargs):

seems to be wrong already - schema is {'__module__': 'builtins', '__qualname__': 'get_x.<locals>.User', 'base_dn': 'ou=people,dc=example,dc=com'} and not the conn object that I would expect.

Do you think that this is a bug, or am I doing something stupid?

[cfelder]

Thanks for reaching out @matejak. Subclassing of the generated modules is definitely something which should work. I'll have a look at this.

[cfelder]

Hello @matejak.

are you running on the latest release 2.7.0, which support subclassing?

I checked your code and it basically works for class User but not for GreenUser which returns a property object for base_dn. I suspect introducing the property base_dn was just a try to workaround the issue.

[cfelder]

looks like you're on 2.6.0

[cfelder]

just note that you may use the Configuration Singleton for storing base_dn as well as custom keys in userconfig dictionary for facilitating the reuse of your code regarding different ldap backend configurations

[matejak]

Thanks for your answer, it is as you have replied - upgrading to 2.7 solved those issues. However, I got to another issues that are still related to subclassing, so I will try to continue in this thread.

[matejak]

Thank you again for your answer and for your tip about the config module. Let me though to share another issues that I have encountered:

The base_dn property: The example that I gave is a bit artificial, so please consider this code:

top_dn = "dc=example,dc=com" 

BaseUser = ldap3_orm.EntryType("uid={uid},{base_dn}", ["inetOrgPerson"], schema=conn)

class ColouredUser(BaseUser):
  def __init__(self, color, * args, ** kwargs):
      self.color = color
      super().__init__(* args, ** kwargs)

  @property
  def base_dn(self):
      ret = f"ou={self.color},ou=people,{top_dn}"
      return ret

user = ColouredUser("grey", uid="johndoe", sn="doe", cn="john")

The code would allow users to be stored in a location that is based on their attribute. However, there seem to be problems with this approach:

1. I can't assign anything (to self.color) before I call the parent's constructor because of an 'ColouredUser' has no attribute '_state' error.
2. But the assignment to doesn't work even if I flip the order because of an entry is read only, cannot add 'color' error.
3. I can add color using ParamDef, and I can propagate its value using the parent's constructor. However, I can't get the color from the base_dn property, {self.color} resolves to an empty string, and user.color too - there is no sign of "grey" or any other color anywhere.
4. Even if I would manage to make base_dn return correct value, it doesn't resolve to a string, but to a reference to a property:
   DN: uid=johndoe\,\<property object at 0x7f8ddfeeb310\> - STATUS: Writable - READ TIME: <never>. Wouldn't it be possible to use the corresponding string?

It looks to me that there are two issues:

A - the derived class inherits the approach that considers all attributes LDAP attributes. I don't think that this is needed, as users are supposed to declare LDAP attributes using the AttrDef.
B - The property isn't converted to its value, or its string representation. Normally, it is quite difficult to get an address of the property object.

Do you think that this is OK as it is, and perhaps should be documented in a section about subclassing, or do you see the current state more as a WIP?
If it is the case of the former, then subclassing may not be the best way, and a more high-level class should be able to return its representation constructed using an EntryType.

[cfelder]

using ParamDef and templating {color} in dn does the job (see also the Automount example).

class ColouredUser(BaseUser):
     dn = "uid={uid},ou={color},{base_dn}"
     base_dn = "ou=people," + top_dn
     color = ldap3_orm.ParamDef("color")

May I have to update the dn documentation to better express that ParamDef is supported too.

The goal of this framework is to have a declarative syntax. Regarding A and B, the current state must not necessarily stay as is but it must be unambitious and documented in which order attributes are resolved. I am open for further discussions and improvements here. I am just wondering about other use cases which would need further attributes on the class, as the current use case is imho well addressed.

Nevertheless resolving the property object I would consider as a bug. I'll have a look at this.

[cfelder]

With commit e6cf598 (branch: dn-properties) properties should now work when specifying dn's

[cfelder]

With commit e86b31a (branch: dn-properties) arbitrary instance variables are allowed. I am still uncertain this is a good idea.

[cfelder]

imho using ParamDef here is the more declarative approach.