On 2017-07-26 13:49 Colm O hEigeartaigh wrote:
Hi Francesco,
On Wed, Jul 26, 2017 at 9:35 AM, Francesco Chicchiriccò
<[email protected]> wrote:
Anyway, your proposed change looks fine, but I would not mix in the
same
field both static and dynamic memberships; hence, I propose to:
* keep List<MembershipTO> memberships
* change List<String> dynGroups into List<MembershipTO>
dynMemberships,
where each MembershipTO has naturally empty attributes lists
Yes, that's the change I was thinking of, let me take care of it! I
have
two somewhat related questions:
a) When creating a user with a membership via the REST API, no
exception is
thrown if the membership key is not valid - the user is just created
without the associated membership. I think it would probably be better
here
to throw an exception...WDYT?
This is more like a general approach, also taken for other cases where,
for example, it is requested to set values for attributes not allowed:
log a warning and discard.
We might want to change this behavior, but then it will affect more than
just memberships.
b) When creating a membership via e.g.:
MembershipTO.Builder().group("9e955aa9-edf1-491c-955a-a9edf1591c7b",
"boss")
the second parameter appears to be ignored by Syncope - you can specify
any
value so long as the key is correct. Again, I think we should consider
throwing an exception here, as otherwise there is no point to
specifying
the group name at all. Maybe we should go further and allow just the
group
name to be specified?
Specifying the group name is more meant as a commodity when it is
Syncope to output MembershipTO instances: if you think this could be
misleading, we might remove the possibility to set the name via
MembershipTO.Builder.
Not sure about requesting membership to be set via group name rather
than via group key: the former is unique but mutable.
Regards.
--
Francesco Chicchiriccò
Tirasa - Open Source Excellence
http://www.tirasa.net/
Member at The Apache Software Foundation
Syncope, Cocoon, Olingo, CXF, OpenJPA, PonyMail
http://home.apache.org/~ilgrosso/
