openstack/keystone
Code Issues Proposed changes

Merge "Faster id mapping lookup"

Browse Source
This commit is contained in:
Jenkins 2016-11-01 00:08:36 +00:00 committed by Gerrit Code Review
commit d8400fe79c
6 changed files with 142 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
keystone/identity/core.py
View File

@ -566,8 +566,8 @@ class Manager(manager.Manager):
return self._set_domain_id_and_mapping_for_single_ref(
ref, domain_id, driver, entity_type, conf)
elif isinstance(ref, list):
return [self._set_domain_id_and_mapping(
x, domain_id, driver, entity_type) for x in ref]
return self._set_domain_id_and_mapping_for_list(
ref, domain_id, driver, entity_type, conf)
else:
raise ValueError(_('Expected dict or list: %s') % type(ref))
@ -576,6 +576,16 @@ class Manager(manager.Manager):
return (driver is not self.driver or not driver.generates_uuids() or
not driver.is_domain_aware())
def _insert_new_public_id(self, local_entity, ref, driver):
# Need to create a mapping. If the driver generates UUIDs
# then pass the local UUID in as the public ID to use.
public_id = None
if driver.generates_uuids():
public_id = ref['id']
ref['id'] = self.id_mapping_api.create_id_mapping(
local_entity, public_id)
LOG.debug('Created new mapping to public ID: %s', ref['id'])
def _set_domain_id_and_mapping_for_single_ref(self, ref, domain_id,
driver, entity_type, conf):
LOG.debug('Local ID: %s', ref['id'])
@ -593,16 +603,60 @@ class Manager(manager.Manager):
LOG.debug('Found existing mapping to public ID: %s',
ref['id'])
else:
# Need to create a mapping. If the driver generates UUIDs
# then pass the local UUID in as the public ID to use.
if driver.generates_uuids():
public_id = ref['id']
ref['id'] = self.id_mapping_api.create_id_mapping(
local_entity, public_id)
LOG.debug('Created new mapping to public ID: %s',
ref['id'])
self._insert_new_public_id(local_entity, ref, driver)
return ref
def _set_domain_id_and_mapping_for_list(self, ref_list, domain_id, driver,
entity_type, conf):
"""Set domain id and mapping for a list of refs.
The method modifies refs in-place.
"""
if not ref_list:
return []
for r in ref_list:
self._insert_domain_id_if_needed(r, driver, domain_id, conf)
if not self._is_mapping_needed(driver):
return ref_list
# build a map of refs for fast look-up
refs_map = {}
for r in ref_list:
refs_map[(r['id'], entity_type, r['domain_id'])] = r
# NOTE(breton): there are cases when the driver is not domain aware and
# no domain_id was explicitely provided for list operation. domain_id
# gets inserted into refs, but not passed into this method. Lets use
# domain_id from one of the refs.
if not domain_id:
domain_id = ref_list[0]['domain_id']
# fetch all mappings for the domain, lookup the user at the map built
# at previous step and replace his id.
domain_mappings = self.id_mapping_api.get_domain_mapping_list(
domain_id)
for _mapping in domain_mappings:
idx = (_mapping.local_id, _mapping.entity_type, _mapping.domain_id)
try:
ref = refs_map.pop(idx)
# due to python specifics, `ref` still points to an item in
# `ref_list`. That's why when we change it here, it gets
# changed in `ref_list`.
ref['id'] = _mapping.public_id
except KeyError:
pass # some old entry, skip it
# at this point, all known refs were granted a public_id. For the refs
# left, there are no mappings. They need to be created.
for ref in refs_map.values():
local_entity = {'domain_id': ref['domain_id'],
'local_id': ref['id'],
'entity_type': entity_type}
self._insert_new_public_id(local_entity, ref, driver)
return ref_list
def _insert_domain_id_if_needed(self, ref, driver, domain_id, conf):
"""Insert the domain ID into the ref, if required.

9
keystone/identity/mapping_backends/base.py
View File

@ -34,6 +34,15 @@ class MappingDriverBase(object):
"""
raise exception.NotImplemented() # pragma: no cover
@abc.abstractmethod
def get_domain_mapping_list(self, domain_id):
"""Return mappings for the domain.
:param domain_id: Domain ID to get mappings for.
:returns: list of mappings.
"""
raise exception.NotImplemented() # pragma: no cover
@abc.abstractmethod
def get_id_mapping(self, public_id):
"""Return the local mapping.

4
keystone/identity/mapping_backends/sql.py
View File

@ -57,6 +57,10 @@ class Mapping(base.MappingDriverBase):
except sql.NotFound:
return None
def get_domain_mapping_list(self, domain_id):
with sql.session_for_read() as session:
return session.query(IDMapping).filter_by(domain_id=domain_id)
def get_id_mapping(self, public_id):
with sql.session_for_read() as session:
mapping_ref = session.query(IDMapping).get(public_id)

42
keystone/tests/unit/test_backend_id_mapping_sql.py
View File

@ -290,3 +290,45 @@ class SqlIDMapping(test_backend_sql.SqlTests):
# Purge mappings the remaining mappings
self.id_mapping_api.purge_mappings({})
self.assertIsNone(self.id_mapping_api.get_public_id(local_entity5))
def test_get_domain_mapping_list(self):
local_id1 = uuid.uuid4().hex
local_id2 = uuid.uuid4().hex
local_id3 = uuid.uuid4().hex
local_id4 = uuid.uuid4().hex
local_id5 = uuid.uuid4().hex
# Create five mappings,two in domainA, three in domainB
local_entity1 = {'domain_id': self.domainA['id'],
'local_id': local_id1,
'entity_type': mapping.EntityType.USER}
local_entity2 = {'domain_id': self.domainA['id'],
'local_id': local_id2,
'entity_type': mapping.EntityType.USER}
local_entity3 = {'domain_id': self.domainB['id'],
'local_id': local_id3,
'entity_type': mapping.EntityType.GROUP}
local_entity4 = {'domain_id': self.domainB['id'],
'local_id': local_id4,
'entity_type': mapping.EntityType.USER}
local_entity5 = {'domain_id': self.domainB['id'],
'local_id': local_id5,
'entity_type': mapping.EntityType.USER}
local_entity1['public_id'] = self.id_mapping_api.create_id_mapping(
local_entity1)
local_entity2['public_id'] = self.id_mapping_api.create_id_mapping(
local_entity2)
local_entity3['public_id'] = self.id_mapping_api.create_id_mapping(
local_entity3)
local_entity4['public_id'] = self.id_mapping_api.create_id_mapping(
local_entity4)
local_entity5['public_id'] = self.id_mapping_api.create_id_mapping(
local_entity5)
# list mappings for domainA
domain_a_mappings = self.id_mapping_api.get_domain_mapping_list(
self.domainA['id'])
domain_a_mappings = [m.to_dict() for m in domain_a_mappings]
self.assertItemsEqual([local_entity1, local_entity2],
domain_a_mappings)

17
keystone/tests/unit/test_backend_ldap.py
View File

@ -3115,6 +3115,23 @@ class DomainSpecificLDAPandSQLIdentity(
domain_scope=self.domains['domain1']['id']),
matchers.HasLength(1))
def test_get_domain_mapping_list_is_used(self):
# before get_domain_mapping_list was introduced, it was required to
# make N calls to the database for N users, and it was slow.
# get_domain_mapping_list solves this problem and should be used
# when multiple users are fetched from domain-specific backend.
for i in range(5):
unit.create_user(self.identity_api,
domain_id=self.domains['domain1']['id'])
with mock.patch.multiple(self.id_mapping_api,
get_domain_mapping_list=mock.DEFAULT,
get_id_mapping=mock.DEFAULT) as mocked:
self.identity_api.list_users(
domain_scope=self.domains['domain1']['id'])
mocked['get_domain_mapping_list'].assert_called()
mocked['get_id_mapping'].assert_not_called()
def test_user_id_comma(self):
self.skip_test_overrides('Only valid if it is guaranteed to be '
'talking to the fakeldap backend')

6
releasenotes/notes/domain_mapping_list-a368ac5a252ec84f.yaml Normal file
View File

@ -0,0 +1,6 @@
---
upgrade:
- ID Mapping driver interface has changed. A new method
``get_domain_mapping_list`` was added for fetching mapping list
for a domain. If you have a custom implementation for the identity
driver, you will need to implement this new method.