Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

call filters: autogenerate name for multi-tenancy purposes #401

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## 23.10

* A call filter now has a read-only auto-generated `name` and a required `label` instead
of a user-provided `name` and optional `label`. For compatibility purposes, if a call filter is
created using only a `name`, it will be used as the `label` and auto-generated.

## 23.04

* PUT on `/users?recursive=true` updated, to provide a way to update lines, switchboards, agent, voicemail and incalls for a specific user.
Expand Down
86 changes: 56 additions & 30 deletions integration_tests/suite/base/test_call_filters.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2018-2019 The Wazo Authors (see the AUTHORS file)
# Copyright 2018-2023 The Wazo Authors (see the AUTHORS file)
# SPDX-License-Identifier: GPL-3.0-or-later

from hamcrest import (
Expand Down Expand Up @@ -38,11 +38,18 @@ def test_put_errors(call_filter):


def error_checks(url):
yield s.check_bogus_field_returns_error, url, 'name', 123
yield s.check_bogus_field_returns_error, url, 'name', None
yield s.check_bogus_field_returns_error, url, 'name', True
yield s.check_bogus_field_returns_error, url, 'name', {}
yield s.check_bogus_field_returns_error, url, 'name', []
Comment on lines -41 to -45
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have any test covering the deprecated way by sending only a name? We still want to support that use case and it needs at least 2 tests (one for success, one for error/invalid data)

yield s.check_bogus_field_returns_error, url, 'label', 123
yield s.check_bogus_field_returns_error, url, 'label', None
yield s.check_bogus_field_returns_error, url, 'label', True
yield s.check_bogus_field_returns_error, url, 'label', {}
yield s.check_bogus_field_returns_error, url, 'label', []
yield s.check_bogus_field_returns_error, url, 'label', 'a' * 129
yield s.check_bogus_field_returns_error, url, 'name', 123, None, 'label'
yield s.check_bogus_field_returns_error, url, 'name', None, None, 'label'
yield s.check_bogus_field_returns_error, url, 'name', True, None, 'label'
yield s.check_bogus_field_returns_error, url, 'name', {}, None, 'label'
yield s.check_bogus_field_returns_error, url, 'name', [], None, 'label'
yield s.check_bogus_field_returns_error, url, 'name', 'a' * 129, None, 'label'
yield s.check_bogus_field_returns_error, url, 'source', 123
yield s.check_bogus_field_returns_error, url, 'source', True
yield s.check_bogus_field_returns_error, url, 'source', 'invalid'
Expand Down Expand Up @@ -78,23 +85,12 @@ def error_checks(url):
yield s.check_bogus_field_returns_error, url, 'enabled', {}
yield s.check_bogus_field_returns_error, url, 'enabled', []

for check in unique_error_checks(url):
yield check


@fixtures.call_filter(name='unique')
def unique_error_checks(url, call_filter):
yield s.check_bogus_field_returns_error, url, 'name', call_filter['name'], {
'strategy': 'all',
'source': 'all',
}


@fixtures.call_filter(name="search", description="SearchDesc")
@fixtures.call_filter(name="hidden", description="HiddenDesc")
@fixtures.call_filter(label="search", description="SearchDesc")
@fixtures.call_filter(label="hidden", description="HiddenDesc")
def test_search(call_filter, hidden):
url = confd.callfilters
searches = {'name': 'search', 'description': 'Search'}
searches = {'label': 'search', 'description': 'Search'}

for field, term in searches.items():
yield check_search, url, call_filter, hidden, field, term
Expand All @@ -110,15 +106,15 @@ def check_search(url, call_filter, hidden, field, term):
assert_that(response.items, is_not(has_item(has_entry('id', hidden['id']))))


@fixtures.call_filter(name="sort1", description="Sort 1")
@fixtures.call_filter(name="sort2", description="Sort 2")
@fixtures.call_filter(label="sort1", description="Sort 1")
@fixtures.call_filter(label="sort2", description="Sort 2")
def test_sorting_offset_limit(call_filter1, call_filter2):
url = confd.callfilters.get
yield s.check_sorting, url, call_filter1, call_filter2, 'name', 'sort'
yield s.check_sorting, url, call_filter1, call_filter2, 'label', 'sort'
yield s.check_sorting, url, call_filter1, call_filter2, 'description', 'Sort'

yield s.check_offset, url, call_filter1, call_filter2, 'name', 'sort'
yield s.check_limit, url, call_filter1, call_filter2, 'name', 'sort'
yield s.check_offset, url, call_filter1, call_filter2, 'label', 'sort'
yield s.check_limit, url, call_filter1, call_filter2, 'label', 'sort'


@fixtures.call_filter(wazo_tenant=MAIN_TENANT)
Expand All @@ -141,6 +137,7 @@ def test_get(call_filter):
response.item,
has_entries(
name=call_filter['name'],
label=call_filter['label'],
source=call_filter['source'],
caller_id_mode=none(),
caller_id_name=none(),
Expand All @@ -166,7 +163,7 @@ def test_get_multi_tenant(main, sub):


def test_create_minimal_parameters():
response = confd.callfilters.post(name='minimal', source='all', strategy='all')
response = confd.callfilters.post(label='minimal', source='all', strategy='all')
response.assert_created('callfilters')

assert_that(response.item, has_entries(id=not_(empty()), tenant_uuid=MAIN_TENANT))
Expand All @@ -176,7 +173,7 @@ def test_create_minimal_parameters():

def test_create_all_parameters():
parameters = {
'name': 'allparameter',
'label': 'all parameters',
'source': 'all',
'strategy': 'all',
'surrogates_timeout': 10,
Expand All @@ -188,7 +185,21 @@ def test_create_all_parameters():

response = confd.callfilters.post(**parameters)
response.assert_created('callfilters')
assert_that(response.item, has_entries(tenant_uuid=MAIN_TENANT, **parameters))
assert_that(
response.item,
has_entries(
tenant_uuid=MAIN_TENANT,
name=not_(empty()),
label='all parameters',
source='all',
strategy='all',
surrogates_timeout=10,
caller_id_mode='prepend',
caller_id_name='callidname',
description='Create description',
enabled=False,
),
)
confd.callfilters(response.item['id']).delete().assert_deleted()


Expand All @@ -207,11 +218,26 @@ def test_create_with_every_enum(self, *call_filters):
pass


def test_create_without_name():
def test_create_without_label():
response = confd.callfilters.post()
response.assert_status(400)


def test_create_with_name_copied_into_label():
response = confd.callfilters.post(
name='test-to-label', source='all', strategy='all'
)
response.assert_created('callfilters')
assert_that(
response.item,
has_entries(
name=is_not('test-to-label'),
label='test-to-label',
),
)
confd.callfilters(response.item['id']).delete().assert_deleted()


@fixtures.call_filter()
def test_edit_minimal_parameters(call_filter):
response = confd.callfilters(call_filter['id']).put()
Expand All @@ -221,7 +247,7 @@ def test_edit_minimal_parameters(call_filter):
@fixtures.call_filter()
def test_edit_all_parameters(call_filter):
parameters = {
'name': 'editallparameter',
'label': 'edit label',
'source': 'all',
'strategy': 'all',
'surrogates_timeout': 10,
Expand Down
5 changes: 4 additions & 1 deletion integration_tests/suite/helpers/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -384,15 +384,17 @@ def insert_paging(self, number='1234', tenant_uuid=None):
def insert_callfilter(
self,
name='bsfilter',
label='boss secretary filter',
type_='bosssecretary',
bosssecretary='secretary-simult',
tenant_uuid=None,
):
query = text(
"""
INSERT INTO callfilter (name, type, bosssecretary, tenant_uuid)
INSERT INTO callfilter (name, label, type, bosssecretary, tenant_uuid)
VALUES (
:name,
:label,
:type,
:bosssecretary,
:tenant_uuid
Expand All @@ -404,6 +406,7 @@ def insert_callfilter(
return self.connection.execute(
query,
name=name,
label=label,
type=type_,
bosssecretary=bosssecretary,
tenant_uuid=tenant_uuid,
Expand Down
22 changes: 11 additions & 11 deletions integration_tests/suite/helpers/helpers/call_filter.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2016-2021 The Wazo Authors (see the AUTHORS file)
# Copyright 2016-2023 The Wazo Authors (see the AUTHORS file)
# SPDX-License-Identifier: GPL-3.0-or-later

import string
Expand All @@ -8,8 +8,8 @@


def generate_call_filter(**params):
name = generate_name()
params.setdefault('name', name)
label = generate_label()
params.setdefault('label', label)
params.setdefault('source', 'all')
params.setdefault('strategy', 'all')
return add_call_filter(**params)
Expand All @@ -26,14 +26,14 @@ def delete_call_filter(call_filter_id, check=False, **kwargs):
response.assert_ok()


def generate_name():
def generate_label():
response = confd.callfilters.get()
names = set(d['name'] for d in response.items)
return _random_name(names)
labels = set(d['label'] for d in response.items)
return _random_label(labels)


def _random_name(names):
name = ''.join(random.choice(string.ascii_lowercase) for i in range(10))
while name in names:
name = ''.join(random.choice(string.ascii_lowercase) for i in range(10))
return name
def _random_label(labels):
label = ''.join(random.choice(string.ascii_lowercase) for i in range(10))
while label in labels:
label = ''.join(random.choice(string.ascii_lowercase) for i in range(10))
return label
10 changes: 9 additions & 1 deletion wazo_confd/plugins/call_filter/api.yml
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,21 @@ definitions:
type: integer
readOnly: true
description: The id of the call filter
uuid:
type: string
description: Call filter UUID. This ID is globally unique across multiple Wazo instances
readOnly: true
tenant_uuid:
type: string
description: The UUID of the tenant
readOnly: true
name:
type: string
description: The name of the call filter
readOnly: true
label:
type: string
description: The label of the call filter
source:
type: string
enum:
Expand Down Expand Up @@ -160,7 +168,7 @@ definitions:
default: true
description: Disable or enable the call filter
required:
- name
- label
- source
- strategy
- $ref: '#/definitions/CallFilterRecipients'
Expand Down
8 changes: 6 additions & 2 deletions wazo_confd/plugins/call_filter/plugin.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# Copyright 2018-2019 The Wazo Authors (see the AUTHORS file)
# Copyright 2018-2023 The Wazo Authors (see the AUTHORS file)
# SPDX-License-Identifier: GPL-3.0-or-later

from xivo_dao.resources.tenant import dao as tenant_dao

from .resource import CallFilterItem, CallFilterList
from .service import build_service

Expand All @@ -10,7 +12,9 @@ def load(self, dependencies):
api = dependencies['api']
service = build_service()

api.add_resource(CallFilterList, '/callfilters', resource_class_args=(service,))
api.add_resource(
CallFilterList, '/callfilters', resource_class_args=(tenant_dao, service)
)

api.add_resource(
CallFilterItem,
Expand Down
21 changes: 19 additions & 2 deletions wazo_confd/plugins/call_filter/resource.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
# Copyright 2018-2023 The Wazo Authors (see the AUTHORS file)
# SPDX-License-Identifier: GPL-3.0-or-later

from flask import url_for
from flask import request, url_for
from uuid import uuid4

from xivo_dao.alchemy.callfilter import Callfilter as CallFilter

Expand All @@ -14,13 +15,29 @@
class CallFilterList(ListResource):
model = CallFilter
schema = CallFilterSchema
call_filter_name_fmt = 'callfilter-{tenant_slug}-{callfilter_uuid}'

def __init__(self, tenant_dao, *args, **kwargs):
super().__init__(*args, **kwargs)
self._tenant_dao = tenant_dao

def build_headers(self, call_filter):
return {'Location': url_for('callfilters', id=call_filter.id, _external=True)}

@required_acl('confd.callfilters.create')
def post(self):
return super().post()
form = self.schema().load(request.get_json())
form = self.add_tenant_to_form(form)

tenant = self._tenant_dao.get(form['tenant_uuid'])
form['uuid'] = uuid4()
form['name'] = self.call_filter_name_fmt.format(
tenant_slug=tenant.slug,
callfilter_uuid=form['uuid'],
)
model = self.model(**form)
model = self.service.create(model)
return self.schema().dump(model), 201, self.build_headers(model)

@required_acl('confd.callfilters.read')
def get(self):
Expand Down
24 changes: 21 additions & 3 deletions wazo_confd/plugins/call_filter/schema.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
# Copyright 2018-2022 The Wazo Authors (see the AUTHORS file)
# Copyright 2018-2023 The Wazo Authors (see the AUTHORS file)
# SPDX-License-Identifier: GPL-3.0-or-later

from marshmallow import fields, post_dump
import logging

from marshmallow import fields, post_dump, pre_load
from marshmallow.validate import OneOf, Length, Range
from xivo.xivo_helpers import clean_extension

from wazo_confd.helpers.mallow import BaseSchema, StrictBoolean, Link, ListLink, Nested

logger = logging.getLogger(__name__)


class CallFilterRecipientsSchema(BaseSchema):
user = Nested(
Expand Down Expand Up @@ -45,8 +49,10 @@ def merge_user(self, data, **kwargs):

class CallFilterSchema(BaseSchema):
id = fields.Integer(dump_only=True)
uuid = fields.UUID(dump_only=True)
tenant_uuid = fields.String(dump_only=True)
name = fields.String(validate=Length(max=128), required=True)
name = fields.String(dump_only=True)
label = fields.String(validate=Length(max=128), required=True)
strategy = fields.String(
validate=OneOf(
[
Expand Down Expand Up @@ -88,3 +94,15 @@ def wrap_users(self, data, **kwargs):
data['surrogates'] = {'users': surrogate_users}

return data

# DEPRECATED 23.10
@pre_load
def copy_name_to_label(self, data, **kwargs):
if 'label' in data:
return data
if 'name' in data:
logger.warning(
'The "name" field of call_filter is deprecated. Use "label" instead'
)
data['label'] = data['name']
return data
Loading