Federation /protocols tests mistakenly use 'protocol_id' instead of 'id'

Bug #1453847 reported by Dolph Mathews
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
python-keystoneclient
Fix Released
Medium
Dolph Mathews

Bug Description

The second paragraph of the keystone-wide API conventions asserts that "each resource contains a canonically unique identifier (ID) defined by the Identity service implementation and is provided as the id attribute":

  https://github.com/openstack/keystone-specs/blob/master/api/v3/identity-api-v3.rst#api-conventions

Following in that spirit, the federation API specification asserts that every instance of a protocol reference should have an 'id' attribute which appears in the URL references as a 'protocol_id':

  https://github.com/openstack/keystone-specs/blob/master/api/v3/identity-api-v3-os-federation-ext.rst#list-all-protocol-and-attribute-mappings-of-an-identity-provider

But the client unit tests create protocol references with a 'protocol_id' attribute instead of an 'id' attribute:

  https://github.com/openstack/python-keystoneclient/blob/91de8422de03e3ef3682f45aebf3ffce90732c5d/keystoneclient/tests/unit/v3/test_federation.py#L144-L148

Fortunately, the server-side implementation appears to persist protocols correctly:

  https://github.com/openstack/keystone/blob/master/keystone/contrib/federation/backends/sql.py#L224-L232

So, 'protocol_id' here should just be 'id', and all the dependent tests need to be revised:

  https://github.com/openstack/python-keystoneclient/blob/91de8422de03e3ef3682f45aebf3ffce90732c5d/keystoneclient/tests/unit/v3/test_federation.py#L147

Confirmed by Steve Martinelli.

Dolph Mathews (dolph)
tags: added: test-improvement
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to python-keystoneclient (master)

Fix proposed to branch: master
Review: https://review.openstack.org/181945

Changed in python-keystoneclient:
status: Confirmed → In Progress
Dolph Mathews (dolph)
description: updated
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to python-keystoneclient (master)

Reviewed: https://review.openstack.org/181945
Committed: https://git.openstack.org/cgit/openstack/python-keystoneclient/commit/?id=3f757656a485d3cec1a56fe4d3f9f07a313a74c7
Submitter: Jenkins
Branch: master

commit 3f757656a485d3cec1a56fe4d3f9f07a313a74c7
Author: Dolph Mathews <email address hidden>
Date: Mon May 11 15:40:33 2015 +0000

    Use 'id' instead of 'protocol_id' in federation protocol tests

    The actual attribute returned in object references of the /protocols API
    is 'id', as in all other keystone APIs that return objects. The
    implementation of new_ref() here doesn't actually include an 'id'
    reference though, and goes out of it's way to test the wrong thing. This
    patch fix that, eliminates the workarounds, and does a touch of
    refactoring to bring these tests in line with tests of other client
    managers.

    Change-Id: I9a272b3ef91934e780106d89b5091b4bfb87ad29
    Closes-Bug: 1453847

Changed in python-keystoneclient:
status: In Progress → Fix Committed
Changed in python-keystoneclient:
milestone: none → 1.4.0
Changed in python-keystoneclient:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.