From 7c94b9f7a4f3770f74d000c39fb41f8b7fab8a4f Mon Sep 17 00:00:00 2001 From: Quentin Bonaventure Date: Tue, 28 Mar 2017 12:09:05 +0200 Subject: [PATCH] DataHandlers documentation --- lodel/leapi/datahandlers/base_classes.py | 364 +++++++++++++++++------ 1 file changed, 279 insertions(+), 85 deletions(-) diff --git a/lodel/leapi/datahandlers/base_classes.py b/lodel/leapi/datahandlers/base_classes.py index 4ebba69..0936d5d 100644 --- a/lodel/leapi/datahandlers/base_classes.py +++ b/lodel/leapi/datahandlers/base_classes.py @@ -1,6 +1,7 @@ #-*- coding: utf-8 -*- - -#  @package lodel.leapi.datahandlers.base_classes Define all base/abstract class for data handlers +## +#  @package lodel.leapi.datahandlers.base_classes Defines all base/abstract +# classes for DataHandlers # # Contains custom exceptions too @@ -30,15 +31,33 @@ LodelContext.expose_modules(globals(), { 'lodel.logger': 'logger', 'lodel.utils.mlstring': ['MlString']}) - -# @brief Base class for all data handlers +## +# @brief Base class for all DataHandlers # @ingroup lodel2_datahandlers +# +# @remarks Some of the methods and properties in this "abstract" class are +# bounded to its children. This implies that the parent +# is aware of its children, which is an absolute anti-pattern +# (Liskov / OC violation), a source of confusion and a decrased +# maintainability. Aggregation =/= Inheritance +# Concerned methods are: is_reference; is_singlereference. +# Concerned properties are __custom_datahandlers; base_handlers. +# @remarks What is the purpose of an internal property being set to a +# string (namely 'automatic') +# @remarks Two sets of methods appears a little strange in regards to their +# visibility. +# - @ref _construct_data / @ref construct_data +# - @ref _check_data_consistency / @ref check_data_consistency class DataHandler(MlNamedObject): base_type = "type" _HANDLERS_MODULES = ('datas_base', 'datas', 'references') - # @brief Stores the DataHandler childs classes indexed by name + + ## + # @brief Stores the DataHandler child classes indexed by name _base_handlers = None - # @brief Stores custom datahandlers classes indexed by name + + ## + # @brief Stores custom DataHandlers classes indexed by name # @todo do it ! (like plugins, register handlers... blablabla) __custom_handlers = dict() @@ -47,17 +66,22 @@ class DataHandler(MlNamedObject): options_spec = dict() options_values = dict() - # @brief List fields that will be exposed to the construct_data_method + ## + # @brief Lists fields that will be exposed to the construct_data method _construct_datas_deps = [] directly_editable = True + + ## # @brief constructor # # @param internal False | str : define whether or not a field is internal - # @param immutable bool : indicates if the fieldtype has to be defined in child classes of + # @param immutable bool : Indicates if the fieldtype has to be defined in child classes of # LeObject or if it is designed globally and immutable - # @throw NotImplementedError if it is instanciated directly + # @throw NotImplementedError If it is instantiated directly + # @remarks Shouldn't the class be declared abstract? No need to check if it + # is instantiated directly, no exception to throw, cleaner code. def __init__(self, **kwargs): if self.__class__ == DataHandler: raise NotImplementedError("Abstract class") @@ -80,9 +104,12 @@ class DataHandler(MlNamedObject): help_text = kwargs.get('help_text', MlString(self.help_text)) super().__init__(display_name, help_text) - # @brief Sets properly casted and checked options for the datahandler - # @raises LodelDataHandlerNotAllowedOptionException when a passed option is not in the option - # specifications of the datahandler + + ## + # @brief Sets properly cast and checked options for the DataHandler + # + # @throw LodelDataHandlerNotAllowedOptionException when a passed option + # is not in the option specifications of the DataHandler def check_options(self): for option_name, option_datas in self.options_spec.items(): if option_name in self.options_values: @@ -96,42 +123,70 @@ class DataHandler(MlNamedObject): # This option was not configured, we get the default value from the specs self.options_values[option_name] = option_datas[0] - # Fieldtype name + + ## + # @return string: Field type name @classmethod def name(cls): return cls.__module__.split('.')[-1] + + ## + # @return bool: True if subclass is of Reference type, False otherwise. @classmethod def is_reference(cls): return issubclass(cls, Reference) + + ## + # @return bool: True if subclass is of SingleRef type, False otherwise. @classmethod def is_singlereference(cls): return issubclass(cls, SingleRef) + + ## + # @return bool: True if the field is a primary_key, False otherwise. def is_primary_key(self): return self.primary_key - # @brief checks if a fieldtype is internal - # @return bool + + ## + # @brief checks if a field type is internal + # @return bool: True if the field is internal, False otherwise. def is_internal(self): return self.internal is not False + + ## # @brief check if a value can be nullable + # # @param value * - # @throw DataNoneValid if value is None and nullable. LodelExceptions if not nullable + # @throw DataNoneValid if value is None and nullable. + # @throw LodelExceptions if not nullable # @return value (if not None) # @return value + # + # @remarks why are there an thrown exception if it is allowed? + # Exceptions are no message brokers def _check_data_value(self, value): if value is None: if not self.nullable: raise LodelExceptions("None value is forbidden for this data field") - raise DataNoneValid("None with a nullable. This exeption is allowed") + raise DataNoneValid("None with a nullable. This exception is allowed") return value + + ## # @brief calls the data_field (defined in derived class) _check_data_value() method # @param value * # @return tuple (value|None, None|error) value can be cast if NoneError + # @remarks Consider renaming this method, such as '_is_data_nullable'. + # @remarks Exceptions ARE NOT message brokers! Moreover, those two methods + # are more complicated than required. In case 'value' is None, + # the returned value is the same as the input value. This is the + # same behavior as when the value is not None! + # @return What's a "NoneError"? Value can be cast to what? def check_data_value(self, value): try: value = self._check_data_value(value) @@ -141,25 +196,36 @@ class DataHandler(MlNamedObject): return None, expt return value, None - # @brief checks if this class can override the given data handler + ## + # @brief Checks if this class can override the given data handler. + # i.e. both class having the same base_type. # @param data_handler DataHandler # @return bool + # @remarks Simplify by "return data_handler.__class__.base_type == self.__class__.base_type"? def can_override(self, data_handler): if data_handler.__class__.base_type != self.__class__.base_type: return False return True + + ## # @brief Build field value + # # @ingroup lodel2_dh_checks - # @warning DO NOT REIMPLEMENT THIS METHOD IN A CUSTOM DATAHANDLER (see # @ref _construct_data() and @ref lodel2_dh_check_impl ) + # # @param emcomponent EmComponent : An EmComponent child class instance # @param fname str : The field name # @param datas dict : dict storing fields values (from the component) # @param cur_value : the value from the current field (identified by fieldname) # @return the value # @throw RunTimeError if data construction fails + # + # @warning DO NOT REIMPLEMENT THIS METHOD IN A CUSTOM DATAHANDLER (see # @todo raise something else + # + # @remarks What the todo up right here means? Raise what? When? + # @remarks Nothing is being raised in this method, should it? def construct_data(self, emcomponent, fname, datas, cur_value): emcomponent_fields = emcomponent.fields() data_handler = None @@ -174,50 +240,71 @@ class DataHandler(MlNamedObject): new_val = None return self._construct_data(emcomponent, fname, datas, new_val) + + ## # @brief Designed to be reimplemented by child classes + # # @param emcomponent EmComponent : An EmComponent child class instance # @param fname str : The field name # @param datas dict : dict storing fields values (from the component) # @param cur_value : the value from the current field (identified by fieldname) # @return the value # @see construct_data() lodel2_dh_check_impl - def _construct_data(self, empcomponent, fname, datas, cur_value): + def _construct_data(self, emcomponent, fname, datas, cur_value): return cur_value - # @brief Check datas consistency + + ## + # @brief Check data consistency # @ingroup lodel2_dh_checks - # @warning DO NOT REIMPLEMENT THIS METHOD IN A CUSTOM DATAHANDLER (see - # @ref _construct_data() and @ref lodel2_dh_check_impl ) - # @warning the datas argument looks like a dict but is not a dict - # see @ref base_classes.DatasConstructor "DatasConstructor" and - # @ref lodel2_dh_datas_construction "Datas construction section" + # + # @ref lodel2_dh_datas_construction "Data construction section" # @param emcomponent EmComponent : An EmComponent child class instance # @param fname : the field name # @param datas dict : dict storing fields values # @return an Exception instance if fails else True + # + # @warning DO NOT REIMPLEMENT THIS METHOD IN A CUSTOM DATAHANDLER (see + # @ref _construct_data() and @ref lodel2_dh_check_impl ) + # @warning the data argument looks like a dict but is not a dict + # see @ref base_classes.DatasConstructor "DatasConstructor" and # @todo A implémenter def check_data_consistency(self, emcomponent, fname, datas): return self._check_data_consistency(emcomponent, fname, datas) + + ## # @brief Designed to be reimplemented by child classes + # # @param emcomponent EmComponent : An EmComponent child class instance # @param fname : the field name # @param datas dict : dict storing fields values # @return an Exception instance if fails else True + # # @see check_data_consistency() lodel2_dh_check_impl def _check_data_consistency(self, emcomponent, fname, datas): return True - # @brief make consistency after a query + + ## + # @brief Makes consistency after a query + # # @param emcomponent EmComponent : An EmComponent child class instance # @param fname : the field name # @param datas dict : dict storing fields values # @return an Exception instance if fails else True - # @todo A implémenter + # + # @todo To be implemented + # @remarks It not clear what is the intent of this method... def make_consistency(self, emcomponent, fname, datas): pass - # @brief This method is use by plugins to register new data handlers + + ## + # @brief Registers a new data handlers + # + # @note Used by plugins. + # @remarks This method is actually never used anywhere. May consider removing it. @classmethod def register_new_handler(cls, name, data_handler): if not inspect.isclass(data_handler): @@ -226,7 +313,9 @@ class DataHandler(MlNamedObject): raise ValueError("A data handler HAS TO be a child class of DataHandler") cls.__custom_handlers[name] = data_handler - # @brief Load all datahandlers + + ## + # @brief Loads all DataHandlers @classmethod def load_base_handlers(cls): if cls._base_handlers is None: @@ -239,22 +328,38 @@ class DataHandler(MlNamedObject): cls._base_handlers[name.lower()] = obj return copy.copy(cls._base_handlers) + + ## # @brief given a field type name, returns the associated python class - # @param fieldtype_name str : A field type name (not case sensitive) + # + # @param name str : A field type name (not case sensitive) # @return DataField child class - # @note To access custom data handlers it can be cool to prefix the handler name by plugin - # name for example ? (to ensure name unicity) + # @throw NameError + # + # @note Would not it be better to prefix the DataHandler name with the + # plugin's one so that it is ensured names are unique? + # @remarks "do/get what from name?" Consider renaming this method (e.g. + # 'get_datafield_from_name') @classmethod def from_name(cls, name): cls.load_base_handlers() all_handlers = dict(cls._base_handlers, **cls.__custom_handlers) name = name.lower() + if name not in all_handlers: raise NameError("No data handlers named '%s'" % (name,)) return all_handlers[name] - # @brief List all datahandlers + + ## + # @brief List all DataHandlers # @return a dict with, display_name for keys, and a dict for value + # @remarks ATM, solely used by the EditorialModel. + # @remarks EditorialModel own class does nothing but calls this class. + # Moreover, nothing calls it anyway. + # @remarks It also seems like it is an EM related concern, and has + # nothing to do with this class. That list appears to be doing + # a purely presentational job. Isn't that a serialization instead? @classmethod def list_data_handlers(cls): cls.load_base_handlers() @@ -269,33 +374,39 @@ class DataHandler(MlNamedObject): return list_dh - # @brief Return the module name to import in order to use the datahandler - # @param data_handler_name str : Data handler name - # @return a str + + ## + # @brief Return the module name to import in order to use the DataHandler + # @param datahandler_name str : Data handler name + # @return str + # @remarks consider renaming this (e.g. "datahandler_module_name") @classmethod - def module_name(cls, name): - name = name.lower() - handler_class = cls.from_name(name) + def module_name(cls, datahandler_name): + datahandler_name = datahandler_name.lower() + handler_class = cls.from_name(datahandler_name) return '{module_name}.{class_name}'.format( module_name=handler_class.__module__, class_name=handler_class.__name__ ) - # @brief __hash__ implementation for fieldtypes + + ## + # @brief __hash__ implementation for field types def __hash__(self): hash_dats = [self.__class__.__module__] for kdic in sorted([k for k in self.__dict__.keys() if not k.startswith('_')]): hash_dats.append((kdic, getattr(self, kdic))) return hash(tuple(hash_dats)) -# @brief Base class for datas data handler (by opposition with references) + +## +# @brief Base class for data data handler (by opposition with references) # @ingroup lodel2_datahandlers - - class DataField(DataHandler): pass +## # @brief Abstract class for all references # @ingroup lodel2_datahandlers # @@ -303,21 +414,31 @@ class DataField(DataHandler): # editorial object # @todo Construct data implementation : transform the data into a LeObject instance class Reference(DataHandler): + base_type = "ref" - - # @brief Instanciation + + ## + # @brief Instantiation # @param allowed_classes list | None : list of allowed em classes if None no restriction - # @param back_reference tuple | None : tuple containing (LeObject child class, fieldname) - # @param internal bool : if False, the field is not internal + # @param back_reference tuple | None : tuple containing (LeObject child class, field name) + # @param internal bool | string: if False, the field is not internal # @param **kwargs : other arguments + # @throw ValueError + # @remarks internal may hold the string value 'automatic'. So far, nothing + # mentions what that means, and nothing seems to be aware + # of an 'automatic' value (at least not in leapi package) def __init__(self, allowed_classes=None, back_reference=None, internal=False, **kwargs): self.__allowed_classes = set() if allowed_classes is None else set(allowed_classes) - # For now usefull to jinja 2 + ## + # @note what is "useful to Jinja 2"? + # For now useful to jinja 2 self.allowed_classes = list() if allowed_classes is None else allowed_classes if back_reference is not None: if len(back_reference) != 2: raise ValueError( "A tuple (classname, fieldname) expected but got '%s'" % back_reference) + ## + # @note Why is there commented out code? Should it be deleted? Ractivated? # if not issubclass(lodel.leapi.leobject.LeObject, back_reference[0]) # or not isinstance(back_reference[1], str): # raise TypeError("Back reference was expected to be a tuple(, str) @@ -325,17 +446,24 @@ class Reference(DataHandler): self.__back_reference = back_reference super().__init__(internal=internal, **kwargs) + + ## # @brief Method designed to return an empty value for this kind of # multipleref + # @remarks purpose!? @classmethod def empty(cls): return None + + ## # @brief Property that takes value of a copy of the back_reference tuple @property def back_reference(self): return copy.copy(self.__back_reference) + + ## # @brief Property that takes value of datahandler of the backreference or # None @property @@ -348,12 +476,17 @@ class Reference(DataHandler): def linked_classes(self): return copy.copy(self.__allowed_classes) - # @brief Set the back reference for this field. + + ## + # @brief Sets a back reference. def _set_back_reference(self, back_reference): self.__back_reference = back_reference - # @brief Check and cast value in appropriate type - # @param value * + + ## + # @brief Check and cast value in the appropriate type + # + # @param value # @throw FieldValidationError if value is an appropriate type # @return value # @todo implement the check when we have LeObject uid check value @@ -369,62 +502,85 @@ class Reference(DataHandler): value = uiddh._check_data_value(value) else: raise FieldValidationError( - "Reference datahandler can not check this value %s if any allowed_class is allowed." % value) + "Reference datahandler can not check this value %s if any allowed_class is allowed. " % value) return value - # @brief Check datas consistency - # @param emcomponent EmComponent : An EmComponent child class instance - # @param fname : the field name + + ## + # @brief Check data consistency + # + # @param emcomponent EmComponent : + # @param fname string : the field name # @param datas dict : dict storing fields values - # @return an Exception instance if fails else True - # @todo check for performance issue and check logics - # @warning composed uid capabilities broken here + # @return bool | Exception : + # + # @todo check for performance issues and checks logic + # @warning composed uid capabilities are broken + # @remarks Is that really a legitimate case of retuning an Exception object? def check_data_consistency(self, emcomponent, fname, datas): rep = super().check_data_consistency(emcomponent, fname, datas) if isinstance(rep, Exception): return rep if self.back_reference is None: return True - # !! Reimplement instance fetching in construct data !! + ## + # @todo Reimplement instance fetching in construct data + # @remarks Set the previous todo as one, looked like it was intended to be. target_class = self.back_reference[0] if target_class not in self.__allowed_classes: logger.warning('Class of the back_reference given is not an allowed class') return False value = datas[fname] - target_uidfield = target_class.uid_fieldname()[0] # multi uid broken here + ## + # @warning multi uid broken here + # @remarks Why is that broken? Any clue? Set as a warning. + target_uidfield = target_class.uid_fieldname()[0] obj = target_class.get([(target_uidfield, '=', value)]) if len(obj) == 0: logger.warning('Object referenced does not exist') return False return True + + ## # @brief Utility method designed to fetch referenced objects + # # @param value mixed : the field value # @throw NotImplementedError + # @remarks Not implemented? Consider renaming? def get_referenced(self, value): raise NotImplementedError -# @brief This class represent a data_handler for single reference to another object +## +# @brief DataHandler for single reference to another object # -# The fields using this data handlers are like "foreign key" on another object +# An instance of this class acts like a "foreign key" to another object class SingleRef(Reference): + def __init__(self, allowed_classes=None, **kwargs): super().__init__(allowed_classes=allowed_classes, **kwargs) - # @brief Check and cast value in appropriate type - # @param value: * - # @throw FieldValidationError if value is unappropriate or can not be cast - # @return value + + ## + # @brief Checks and casts value to the appropriate type + # + # @param value: mixed + # @throw FieldValidationError if value is inappropriate or can not be cast + # @return mixed def _check_data_value(self, value): value = super()._check_data_value(value) return value - # @brief Utility method designed to fetch referenced objects + + ## + # @brief Utility method to fetch referenced objects + # # @param value mixed : the field value # @return A LeObject child class instance # @throw LodelDataHandlerConsistencyException if no referenced object found + # @remarks Consider renaming (e.g. get_referenced_object)? def get_referenced(self, value): for leo_cls in self.linked_classes: res = leo_cls.get_from_uid(value) @@ -434,30 +590,39 @@ class SingleRef(Reference): referenced object with uid %s" % value) -# @brief This class represent a data_handler for multiple references to another object +## +# @brief DataHandler for multiple references to another object # @ingroup lodel2_datahandlers # -# The fields using this data handlers are like SingleRef but can store multiple references in one field +# The fields using this data handlers are like SingleRef but can store multiple +# references in one field. # @note for the moment split on ',' chars class MultipleRef(Reference): + ## # @brief Constructor - # @param max_item int | None : indicate the maximum number of item referenced by this field, None mean no limit + # + # @param max_item int | None : indicate the maximum number of item referenced + # by this field, None mean no limit def __init__(self, max_item=None, **kwargs): self.max_item = max_item super().__init__(**kwargs) + ## # @brief Method designed to return an empty value for this kind of - # multipleref + # multipleref + # @remarks Purpose!? @classmethod def empty(cls): return [] + + ## # @brief Check and cast value in appropriate type - # @param value * + # @param value mixed # @throw FieldValidationError if value is unappropriate or can not be cast # @return value - # @TODO Writing test error for errors when stored multiple references in one field + # @todo Writing test error for errors when stored multiple references in one field def _check_data_value(self, value): value = DataHandler._check_data_value(self, value) if not hasattr(value, '__iter__'): @@ -479,7 +644,9 @@ class MultipleRef(Reference): "MultipleRef have for invalid values [%s] :" % (",".join(error_list))) return new_val + ## # @brief Utility method designed to fetch referenced objects + # # @param values mixed : the field values # @return A list of LeObject child class instance # @throw LodelDataHandlerConsistencyException if some referenced objects @@ -502,37 +669,51 @@ class MultipleRef(Reference): some referenced objects. Following uids were not found : %s" % ','.join(left)) -#  @brief Class designed to handle datas access will fieldtypes are constructing datas +## +# @brief Class designed to handle data access while field types are constructing data # @ingroup lodel2_datahandlers # # This class is designed to allow automatic scheduling of construct_data calls. # -# In theory it's able to detect circular dependencies +# In theory it has the ability to detect circular dependencies # @todo test circular deps detection # @todo test circular deps false positive +# @remarks Would not it be better to make sure what the code actually is doing? class DatasConstructor(object): + ## # @brief Init a DatasConstructor - # @param leobject LeCrud : @ref LeObject child class + # + # @param leobject LeObject # @param datas dict : dict with field name as key and field values as value # @param fields_handler dict : dict with field name as key and data handler instance as value def __init__(self, leobject, datas, fields_handler): - # Stores concerned class self._leobject = leobject - # Stores datas and constructed datas self._datas = copy.copy(datas) # Stores fieldtypes self._fields_handler = fields_handler - # Stores list of fieldname for constructed datas + # Stores list of fieldname for constructed self._constructed = [] # Stores construct calls list self._construct_calls = [] + + ## # @brief Implements the dict.keys() method on instance + # + # @return list def keys(self): return self._datas.keys() - #  @brief Allows to access the instance like a dict + + ## + # @brief Allows to access the instance like a dict + # + # @param fname string: The field name + # @return field values + # @throw RuntimeError + # + # @note Determine return type def __getitem__(self, fname): if fname not in self._constructed: if fname in self._construct_calls: @@ -543,17 +724,25 @@ class DatasConstructor(object): self._constructed.append(fname) return self._datas[fname] + + ## # @brief Allows to set instance values like a dict + # # @warning Should not append in theory + # + # @remarks Why is a warning issued any time we call this method? def __setitem__(self, fname, value): self._datas[fname] = value warnings.warn("Setting value of an DatasConstructor instance") -# @brief Class designed to handle an option of a DataHandler +## +# @brief Class designed to handle a DataHandler option class DatahandlerOption(MlNamedObject): - # @brief instanciates a new Datahandler option object + + ## + # @brief instantiates a new DataHandlerOption object # # @param id str # @param display_name MlString @@ -564,13 +753,18 @@ class DatahandlerOption(MlNamedObject): self.__validator = validator super().__init__(display_name, help_text) + ## + # @brief Accessor to the id property. @property def id(self): return self.__id + ## # @brief checks a value corresponding to this option is valid - # @param value - # @return casted value + # + # @param value mixed + # @return cast value + # @throw ValueError def check_value(self, value): try: return self.__validator(value)