Bug 42478 - Simplify UsernameHandler.format_username() and increase robustness
Simplify UsernameHandler.format_username() and increase robustness
Product: UCS@school
Classification: Unclassified
Component: Import scripts
UCS@school 4.1 R2
Other Linux
: P5 normal (vote)
: UCS@school 4.1 R2 vXXX
Assigned To: Florian Best
Daniel Tröder
: interim-3
Depends on:
  Show dependency treegraph
Reported: 2016-09-22 17:02 CEST by Florian Best
Modified: 2016-12-12 13:10 CET (History)
2 users (show)

See Also:
What kind of report is it?: Development Internal
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:

patch (7.88 KB, patch)
2016-09-22 17:02 CEST, Florian Best
Details | Diff
cover_username_handler.py (652 bytes, text/x-python)
2016-11-10 18:26 CET, Florian Best

Note You need to log in before you can comment on or make changes to this bug.
Description Florian Best univentionstaff 2016-09-22 17:02:15 CEST
Created attachment 8027 [details]

The rewritten function makes things like:
* strip('.')
* remove_bad_chars()
only once instead of for each case once.

It raises always an error if the username was shortened to an empty string (not only in one special case).

It has a mccabe complexity of 7 instead of 14.

It raises already if two [ALWAYSCOUNTER] patterns are used instead of 3.

It allows [foo] in the username not to raise an error but actually leave it this way (the [ ] are replaced by the bad character removal).

The maxlength is only substracted by 3 len('999') if a counter is used.

For each case a doctest is written. It has 100% coverage.

Another thing in the original function is that:
Will use the same number. Resulting in username like:

But I kept this because we already released this and there might be existing LDAP entries which needed to be cared about in the counter functions.
The best idea would be to replace the [..] part by an unique string.
Comment 1 Florian Best univentionstaff 2016-09-22 17:13:56 CEST
The function doesn't produce illegal usernames anymore. Currently it might produces the following:

>>> UsernameHandler(5).format_username('abcd.ef')
Username 'abcd.ef' too long, shortened to 'abcd.'.

→ A username with trailing dot
Comment 2 Florian Best univentionstaff 2016-10-19 15:44:17 CEST
*** Bug 42472 has been marked as a duplicate of this bug. ***
Comment 3 Florian Best univentionstaff 2016-11-10 18:10:11 CET
ucs-school-import (14.0.16-33):
r74308 | Bug #42478: Simplify UsernameHandler.format_username() and increase robustness

r74309 | YAML Bug #42478
Comment 4 Florian Best univentionstaff 2016-11-10 18:26:14 CET
Created attachment 8215 [details]

For the 100% coverage of the docstring:

# apt-get install python-coverage
# python-coverage run cover_username_handler.py
# python-coverage report | grep username_handler
/usr/share/pyshared/ucsschool/importer/utils/username_handler                         78     13    83%
cover_username_handler                                                                17      0   100%
→ the 17% missing percent are the overridden methods which would do LDAP interaction
# python-coverage html
firefox htmlcov/_usr_share_pyshared_ucsschool_importer_utils_username_handler.html
Comment 5 Daniel Tröder univentionstaff 2016-11-13 21:50:13 CET
r74370: fixed 90_ucsschool/215_import-users_illegal_chars_in_username_v2
r74372: add test with moved COUNTER variable to 90_ucsschool/215_import-users_illegal_chars_in_username

OK: code (very concise:)
OK: automated tests:
  * 90_ucsschool/113_import_factory_configurability::test_username_handler()
  * 90_ucsschool/215_import-users_illegal_chars_in_username
  * 90_ucsschool/215_import-users_illegal_chars_in_username_v2
OK: "[..]" other than those in counter_variable_to_function.keys() don't lead to an error anymore
OK: correct handling of username length and dot-at-ends
OK: [VARIABLE] can move inside scheme:username (as long as the rest of the template isn't changed) and the counter keeps being used (nice!)
Comment 6 Daniel Tröder univentionstaff 2016-11-14 07:40:22 CET
(In reply to Daniel Tröder from comment #5)
> r74370: fixed 90_ucsschool/215_import-users_illegal_chars_in_username_v2
> r74372: add test with moved COUNTER variable to
> 90_ucsschool/215_import-users_illegal_chars_in_username
Forgot to build it yesterday - won't be in jenkins today.
Comment 7 Daniel Tröder univentionstaff 2016-11-14 12:37:20 CET
OK: r74388 + 74389 (remove duplicated code, advisory update)
Comment 8 Sönke Schwardt-Krummrich univentionstaff 2016-12-12 13:10:19 CET
UCS@school 4.1 R2 v9 has been released.