Bug 41469 - Factory configurable
Factory configurable
Status: RESOLVED FIXED
Product: UCS@school
Classification: Unclassified
Component: Import scripts
UCS@school 4.1 R2
Other Linux
: P5 normal (vote)
: UCS@school 4.1 R2
Assigned To: Daniel Tröder
Sönke Schwardt-Krummrich
: interim-1
: 41470 (view as bug list)
Depends on: 41239
Blocks:
  Show dependency treegraph
 
Reported: 2016-06-06 11:36 CEST by Jan Christoph Ebersbach
Modified: 2016-09-30 11:59 CEST (History)
2 users (show)

See Also:
What kind of report is it?: Feature Request
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional):
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Christoph Ebersbach univentionstaff 2016-06-06 11:36:03 CEST
In my tests of the new ucs@school-import-tool, Bug #41239, I discoverd that the factory "DefaultFactory" has to be forked for every single extension of the import mechanism.  This is suboptimal because it requires lots of additional effort and increases maintenance costs.  For example adding a hook for teacher accounts produces requires two derived classes in order to be able to implement the hook method.
Comment 1 Daniel Tröder univentionstaff 2016-06-06 15:28:09 CEST
Forking the factory is required only once per project / source database (not per extension). It requires 5 lines of code. All changes should be done in the overridden UserImport.[pre]_[create]_hook().

Anyway, a hook mechanism has been implemented that will search for files like the legacy mechanism does. The files are Python classes though.

I didn't see this Bug before, so I wrote it in https://forge.univention.org/bugzilla/show_bug.cgi?id=41239#c5 and copy it now here:

With r69848, there are now 3 types of hooks, that are run before and after adding, modifying and deleting user objects:
* (A, legacy) arbitrary executables (with certain names) can be stored in directories below /usr/share/ucs-school-import/hooks/ and are run by ucs-school-lib
* (B) ucsschool.importer.mass_import.user_import.UserImport can be subclassed and pre_create_hook(), post_modify_hook() etc can be overridden
* (C) Python files containing subclasses of ucsschool.importer.utils.pyhook.PyHook can be stored in directories below /usr/share/ucs-school-import/pyhooks/ and their run() method is run by ucs-school-import (in UserImport) in alphabetical order of the filenames.

The order in which hooks are executed is:

B → C → A → add/mod/del → A → C → B

B and C have full access to the ImportUser instance. Changes done at "pre" time will be saved to LDAP. "post"-code could also do user.modify(), but that is discouraged.
Comment 2 Daniel Tröder univentionstaff 2016-06-06 15:30:27 CEST
*** Bug 41470 has been marked as a duplicate of this bug. ***
Comment 3 Daniel Tröder univentionstaff 2016-06-06 15:34:13 CEST
Not related, but may be of interest:

The methods of the abstract factory (actually the default_factory) do not have to be overwritten be subclassing anymore. The default_factory does auto-load configured classes or functions since r69788.
The configuration is documented in /usr/share/doc/ucs-school-import/configuration_readme.txt.gz
Comment 4 Daniel Tröder univentionstaff 2016-06-07 14:23:52 CEST
(In reply to Daniel Tröder from comment #3)
> Not related, but may be of interest:
> The methods of the abstract factory (actually the default_factory) do not
> have to be overwritten be subclassing anymore. The default_factory does
> auto-load configured classes or functions since r69788.
> The configuration is documented in
> /usr/share/doc/ucs-school-import/configuration_readme.txt.gz

An example:

----- /var/lib/ucs-school-import/configs/myconfig.json ----------------

config = {
  ..,
  "classes": {
    "reader": "myschool.myreader.MyReader"
  }
}

----- /usr/share/pyshared/myschool/myreader.py ------------------------

from ucsschool.importer.reader.csv_reader import CsvReader

class MyReader(CsvReader):
  def handle_input(self, mapping_key, mapping_value, csv_value, import_user):
    if mapping_value == "mymobile":
      import_user.udm_properties["phone"] = "+49-{}".format(csv_value)
      return True
    return super(MyReader, self).handle_input(mapping_key, mapping_value, csv_value, import_user)

-----------------------------------------------------------------------

Now in the import-code wherever factory.make_reader() is called, a MyReader instance ist created.
Comment 5 Daniel Tröder univentionstaff 2016-06-07 14:48:42 CEST
(In reply to Daniel Tröder from comment #1)
> * (B) ucsschool.importer.mass_import.user_import.UserImport can be
> subclassed and pre_create_hook(), post_modify_hook() etc can be overridden

An example:

----- /var/lib/ucs-school-import/configs/myconfig.json ----------------

config = {
  ..,
  "classes": {
    "user_importer": "myschool.user_importer.MyUserImporter"
  }
}

----- /usr/share/pyshared/myschool/user_importer.py ------------------------

from ucsschool.importer.mass_import.user_import import UserImport

class MyUserImporter(UserImport):
  def pre_create_hook(self, user):
    fnames = user.firstname.split()
    if len(fnames) > 1:
      user.firstname = "{} {}.".format(fnames[0], ". ".join([n[0] for n in fnames[1:]]))
Comment 6 Daniel Tröder univentionstaff 2016-06-07 14:53:27 CEST
(In reply to Daniel Tröder from comment #1)
> * (C) Python files containing subclasses of
> ucsschool.importer.utils.pyhook.PyHook can be stored in directories below
> /usr/share/ucs-school-import/pyhooks/ and their run() method is run by
> ucs-school-import (in UserImport) in alphabetical order of the filenames.

An example:

-- /usr/share/ucs-school-import/pyhooks/user_create_post.d/static_birthday.py --

from ucsschool.importer.utils.pyhook import PyHook

class AllCelebrateTogether(PyHook):
  def run(self):
    self.import_user.birthday = "1999-01-23"
Comment 7 Daniel Tröder univentionstaff 2016-06-16 10:20:17 CEST
Two examples for extending the functionality now exist:
a) /usr/share/doc/ucs-school-import/hook_example.py
b) /usr/share/doc/ucs-school-import/subclassing_example.py

a) python based hooks
---------------------
Python hooks have been changed drastically! They are now created as a class containing all hooks for one object type (user, computer, ..), subclassing a specific subclass of PyHook. Currently only UserPyHook exist.
In such a class methods pre_create(user), post_create(user) etc. can be implemented. 'user' is a [Legacy[Import]]User.
If multiple classes are found, the 'priority' attribute is used to define the execution order. If priority["post_create"] == None (the default), a method is not run at all.

b) subclassing
--------------
An example for Comment #5 was added, including a lengthy description how to set it up: subclassing_example.py
Comment 8 Florian Best univentionstaff 2016-07-21 14:55:11 CEST
I don't see any commits for this bug.
Comment 9 Daniel Tröder univentionstaff 2016-09-21 17:50:15 CEST
r72734: added test 90_ucsschool/113_import_factory_configurability that one after another tests:

"classes": {
	"reader": "univention.testing.ucsschool_import_factory_test_classes.TypeCsvReader",
	"mass_importer": "univention.testing.ucsschool_import_factory_test_classes.NullImport",
	"password_exporter": "univention.testing.ucsschool_import_factory_test_classes.UniventionPasswordExporter",
	"result_exporter": "univention.testing.ucsschool_import_factory_test_classes.AnonymizeResultExporter",
	"user_importer": "univention.testing.ucsschool_import_factory_test_classes.BirthdayUserImport",
	"username_handler": "univention.testing.ucsschool_import_factory_test_classes.FooUsernameHandler",
	"user_writer": "univention.testing.ucsschool_import_factory_test_classes.JsonWriter"
}

In all cases it uses classes:reader=TypeCsvReader.
Comment 10 Florian Best univentionstaff 2016-09-23 16:55:28 CEST
http://jenkins.knut.univention.de:8080/job/UCSschool%204.1/job/UCSschool%204.1%20(R2)%20Multiserver/lastCompletedBuild/SambaVersion=s4-only-master/testReport/90_ucsschool/70_users_module/test/

(2016-09-22 19:54:22.636842) Traceback (most recent call last):
(2016-09-22 19:54:22.636897)   File "113_import_factory_configurability", line 307, in <module>
(2016-09-22 19:54:22.637001)     fct.run_all_tests()
(2016-09-22 19:54:22.637047)   File "113_import_factory_configurability", line 294, in run_all_tests
(2016-09-22 19:54:22.637141)     self.test_password_exporter()
(2016-09-22 19:54:22.637185)   File "113_import_factory_configurability", line 124, in test_password_exporter
(2016-09-22 19:54:22.637260)     "output:new_user_passwords={}".format(outfile.name)])
(2016-09-22 19:54:22.637302)   File "113_import_factory_configurability", line 88, in run_import
(2016-09-22 19:54:22.637371)     raise Exception('Non-zero exit code %r' % (exitcode,))
(2016-09-22 19:54:22.637472) Exception: Non-zero exit code 1
(2016-09-22 19:54:22.691723) Exception OSError: (2, 'No such file or directory', '/tmp/113factest.yZdvwY/tmpkIkLjN') in <bound method _TemporaryFileWrapper.__del__ of <closed file '<fdopen>', mode 'w+b' at 0x2168f60>> ignored
(2016-09-22 19:54:22.691801) Exception OSError: (2, 'No such file or directory', '/tmp/113factest.yZdvwY/tmpP65Vut') in <bound method _TemporaryFileWrapper.__del__ of <closed file '<fdopen>', mode 'w+b' at 0x275db70>> ignored
Comment 11 Florian Best univentionstaff 2016-09-23 16:56:48 CEST
http://jenkins.knut.univention.de:8080/job/UCSschool%204.1/job/UCSschool%204.1%20(R2)%20Multiserver/lastCompletedBuild/SambaVersion=s4-only-master/testReport/90_ucsschool/112_import_user_pyhooks/test/

(2016-09-22 19:53:40.038500) Traceback (most recent call last):
(2016-09-22 19:53:40.038545)   File "112_import_user_pyhooks", line 25, in <module>
(2016-09-22 19:53:40.038617)     shutil.copy2(TESTHOOKSOURCE, TESTHOOKTARGET)
(2016-09-22 19:53:40.038665)   File "/usr/lib/python2.7/shutil.py", line 130, in copy2
(2016-09-22 19:53:40.038741)     copyfile(src, dst)
(2016-09-22 19:53:40.038784)   File "/usr/lib/python2.7/shutil.py", line 82, in copyfile
(2016-09-22 19:53:40.038851)     with open(src, 'rb') as fsrc:
(2016-09-22 19:53:40.038955) IOError: [Errno 2] No such file or directory: '/root/sync/ucs-test-ucsschool/90_ucsschool/testpyhookpy'
Comment 12 Daniel Tröder univentionstaff 2016-09-26 08:22:48 CEST
Already fixed that morning in r72768.