[Freeipa-devel] Return values, CRUD, webUI

[Jason Gerard DeRose]

The vast majority of our Command plugins subclass from one of the CRUD
base classes, so in terms of return value consistency and API style, we
need to focus most on them (and then adapt their style to the few
non-CRUD commands).

While hooking up the webUI there have been many, many small problems in
the core library and plugins that have caused unexpected setbacks for
me.  Some features that I needed got changed without me noticing, some
of my half-baked designs needed more baking, some features were missing,
and some new code I was just unfamiliar with.  Point is, I've spent a
lot of time battling little gotchas and thinking about how best to clean
these things up.  Here are the guidelines I propose we follow:


A return value dict
===================

As much as possible, I want to keep our return values very simple and
regular.  This 1) makes our API easy to learn and use, and 2) makes it
easy to use the return values to drive UI logic on both the CLI and
webUI.

One current source of irregularity is the need to pass the "this isn't
all the entries" flag from LDAP when we do searches.  For example,
`user_find` returns an (entry_list, more_remains) tuple.  The problem is
that most of the code paths don't care about the `more_remains` flag...
they just need to know whether a list of entries was returned (result is
a list) or whether a single entry was returned (result is a dict).

At the same time, we obviously need a way to pass extra data like the
`more_remains` flag and it would be nice to be able to extend a return
value with additional special data without breaking code or causing an
explosion of special cases.  So I propose that our return values always
be a dict containing at least a 'result', leaving us the option to
extend the return value without breaking code that just looks at
ret['result'].

So in the case of a search, instead of:

    ([{'uid': 'foo'}, {'uid': 'bar'}, ...], True)

We should return:

    {
        'result': [{'uid': 'foo'}, {'uid': 'bar'}, ...],
        'more_remains': True,
        ...
        'extend': 'as-needed',
    }

The following all assume we are returning {'result': blah} even though
they don't show it...


Entries
=======

95% of our return values are LDAP entries.  Currently we're returning
pretty much the raw value from python-ldap (although we are decoding
UTF-8 into `unicode` objects for use in the Python pipeline and encoding
back to UTF-8 on the way out, which is good).  But the data structure
returned from python-ldap is pretty awkward to work with.

First, at the top it's typically a (dn, entry) tuple.  Assuming the 'dn'
key doesn't conflict with any sane LDAP attribute names, I think we
should return a single dict with the dn stored under the 'dn' key.

So instead of:

    ('uid=jdoe,cn=users,cn=accounts,dc=example,dc=com', {'sn': ['Doe']})

We should return:

    {'dn': 'uid=jdoe,cn=users,cn=accounts,dc=example,dc=com', 'sn': ['Doe']}

Second, currently we return all attribute values inside a list whether
or not they're multi-value.  This leads to lots of special cases
throughout the code that would be better dealt with in a single place,
in LDAP Backend adapter, IHMO.

So instead of:

    {'uid': ['jdoe'], 'group': ['foo', 'bar']}

We should return:

    {'uid': 'jdoe', 'group': ['foo', 'bar']}


Lists of Entries
================

When a command returns multiple entries, the entries should be in the
same form as they are from commands that return only one entry.  For
example, currently user-find returns each entry as a (uid, entry) tuple.
I think this should again be replaced with a single dict without the uid
being duplicated.


Create
======

If successful, we should return the resulting entry in standard form.
If any error occurs, we should raise an appropriate exception.


Retrieve
========

If successful, we should return the entry in standard form.  If no such
entry exists we should raise a NotFound exception.  If any other error
occurs, we should raise an appropriate exception.


Update
======

(Same as Create.)


Delete
======

(Same as Retrieve.)


Search
======

If one or more entries matches the search criteria, we should return a
list of entries, where the each entry is in standard form.  If no
entries match, we should return an empty list.  If an error occurs, we
should raise an appropriate exception.


Thoughts?

-Jason