If you're going to allow clients to POST null values, you need to check them in api/positions.py as well.
In particular, you have a snippet of code which does this:
If the body has a required position set to null, this check will fail to catch that.
It might be easier to simply remove null entries from the body at the very beginning.
If you're going to allow clients to POST null values, you need to check them in api/positions.py as well.
In particular, you have a snippet of code which does this:
```python
for position in required:
if position not in body:
return {
'error': f'missing required position: {position}'
}, 400
```
If the body has a required position set to null, this check will fail to catch that.
It might be easier to simply remove null entries from the body at the very beginning.
The API should return a 400 error for a request like this.
Here's an easy fix: change
ifpositionnotinbody:
to
ifnotbody.get(position):
This sort of thing is now possible:
```
curl --negotiate -u : --service-name ceod --delegation always -X POST \
-d '{"president":"ctdalek","sysadmin":null,"vice-president":null}' \
http://phosphoric-acid:9987/api/positions
```
The API should return a 400 error for a request like this.
Here's an easy fix: change
```python
if position not in body:
```
to
```python
if not body.get(position):
```
Added unit tests. I changed more provideUtility with registerUtility() so the dynamic position parameter works correctly. (It seems like it's where the library want people to use too judging by this comment 5bd246ed53/src/zope/component/globalregistry.py (L66))
Added unit tests. I changed more provideUtility with registerUtility() so the dynamic position parameter works correctly. (It seems like it's where the library want people to use too judging by this comment https://github.com/zopefoundation/zope.component/blob/5bd246ed532d8ace5034bb09777b9f5b8eb87150/src/zope/component/globalregistry.py#L66)
zope's registerUtility emits an IUtilityRegistration containing the component that got registered. Here I'm just checking for that, and whether the registered component is an IConfig.
zope's `registerUtility` emits an IUtilityRegistration containing the component that got registered. Here I'm just checking for that, and whether the registered component is an IConfig.
merenber
Closes #9
One change from #8 is that the
getsubcommand is not optional, makes it easier to implement using click.Looking good - just a few minor adjustments.
Also, we still need CLI unit tests.
@positions.command(short_help='Update positions')def update(**kwargs):I think that this should be called 'set' instead of 'update'.
required = cfg.get('positions_required')for pos in avail:update = click.option(f'--{pos}', metavar='USER', required=(pos in required))(update)metavar should be 'USERNAME'.
Also, users should be prompted for required positions (set
prompt=True).self.positions = defaultdict(list)for position, username in positions_reversed.items():self.positions[username].append(position)if username is not None:If you're going to allow clients to POST null values, you need to check them in api/positions.py as well.
In particular, you have a snippet of code which does this:
If the body has a required position set to null, this check will fail to catch that.
It might be easier to simply remove null entries from the body at the very beginning.
the change doesn't allow positions to be null, only usernames. So it should handle it fine.
This sort of thing is now possible:
The API should return a 400 error for a request like this.
Here's an easy fix: change
to
ah I see what you mean. Will fix
6b289a1ec4to5bae89a9fd2 years agoAdded unit tests. I changed more provideUtility with registerUtility() so the dynamic position parameter works correctly. (It seems like it's where the library want people to use too judging by this comment
5bd246ed53/src/zope/component/globalregistry.py (L66))def _handler(event):global setif not (IUtilityRegistration.providedBy(event.object) and IConfig.providedBy(event.object.component)):What is this doing?
zope's
registerUtilityemits an IUtilityRegistration containing the component that got registered. Here I'm just checking for that, and whether the registered component is an IConfig.The reason why this whole thing is here is to make the parameters (--president, --syscom, etc) follow what is in the config.
After I merged v1 in I couldn't get the zope callback to work anymore, so I just created an explicit funtion which is called in positions().
651988bb08into v1 2 years agoReviewers
651988bb08.