~adam-collard/click-reviewers-tools/store-requests-category

Viewing changes to clickreviews/sr_declaration.py

Committer: Jamie Strandboge
Date: 2019-04-03 22:43:41 UTC
Revision ID: jamie@ubuntu.com-20190403224341-yn5t738olnbv0o8v

sr_declaration.py: fix verification interface attributes that are lists

Due to an ambiguity in the snap declaration interfaces spec, snapd implemented
interface attributes one way and the review-tools another, which has caused
some iterations on this check.

Fix a regression introduced in r1208 that broke interface attribute tests when
the attribute is a list, and fix the testsuite to catch this in the future (the
tests incorrectly mocked the snap.yaml as strings and the snap decl as lists
when it should have been the other way around, so this wasn't caught).

Don't support lists in the declaration for interface attributes since while
tecnically supported via recursion in snapd, they are impractical and unused in
practice. Add a clarifying comment for this.

Remove some tests introduced in r1208 to test this impractical condition since
the previous code was wrong and the tests impractical. Only test that we raise
an exception in this impractical case.

files modified:
clickreviews/sr_declaration.py

clickreviews/tests/test_sr_declaration.py

Show diffs side-by-side

added added

removed removed

clickreviews/sr_declaration.py

454

if type(val) not in [str, list, dict, bool]:

455

raise SnapDeclarationException("unknown type '%s'" % val)

456

457

# Keep in mind by this point each OR constraint is being iterated

458

# through such that 'against' as a list is a nested list.

459

# For now, punt on nested lists since they are impractical in use

460

# since they are a list of OR options where one option must match

461

# all of val, but if you have that you may as well just use a

462

# string. Eg for this snap.yaml:

463

# snap.yaml:

464

# plugs:

465

# foo:

466

# bar: [ baz, norf ]

467

468

# a snap decl that uses a list for 'bar' might be:

469

# snap decl:

470

# foo:

471

# plug-attributes:

472

# bar:

473

# - baz|norf

474

# - something|else

475

476

# but, 'something|else' is pointless so it will never match, so the

477

# decl should be rewritten more simply as:

478

# snap decl:

479

# foo:

480

# plug-attributes:

481

# bar: baz|norf

482

# Importantly, the type of the attribute in the snap decl means

483

# something different than the type in snap.yaml

484

if isinstance(against, list):

485

raise SnapDeclarationException(

486

"attribute lists in the declaration not supported")

487

457

488

matched = False

458

if isinstance(val, str):

489

if isinstance(val, str) and isinstance(against, str):

459

490

if against.startswith('$'):

460

491

if against == '$MISSING':

461

492

matched = False # value must not be set

469

500

elif re.search(r'^(%s)$' % against, val):

470

501

matched = True

471

502

elif isinstance(val, list):

472

# if the attribute in the snap (val) is a list, then to match,

473

# the declaration value (against) must also be a list and the

474

# unordered contents of the lists must be the same

475

if isinstance(against, list) and set(val) == set(against):

476

matched = True

503

# if the attribute in the snap (val) is a list and the

504

# declaration value (against) is a string, then to match,

505

# against must be a regex that matches all entries in val

506

for i in val:

507

if _check_attrib(i, against, side, rules_attrib):

508

matched = True

477

509

else: # bools and dicts (TODO: nested matches for dicts)

478

510

matched = (against == val)

479

511

Older »