From 04afcda9ff8602991c3d078058711c8b922c0cd0 Mon Sep 17 00:00:00 2001 From: Evan Marell Date: Mon, 19 Mar 2018 15:34:32 -0700 Subject: [PATCH] Refactor `#clean_old_tokens` to reduce computational complexity. * Previous version featured an `Enumerable#min_by` loop _inside_ a `while` loop, resulting in `O(n^2)` complexity. * Instead, break things into two separate loops, and skip altogether if they aren't even necessary. --- app/models/devise_token_auth/concerns/user.rb | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/app/models/devise_token_auth/concerns/user.rb b/app/models/devise_token_auth/concerns/user.rb index 142608803..214374dc0 100644 --- a/app/models/devise_token_auth/concerns/user.rb +++ b/app/models/devise_token_auth/concerns/user.rb @@ -258,9 +258,17 @@ def max_client_tokens_exceeded? end def clean_old_tokens - while tokens.present? && max_client_tokens_exceeded? - oldest_client_id, _tk = tokens.min_by { |_cid, v| v[:expiry] || v["expiry"] } - tokens.delete(oldest_client_id) + if tokens.present? && max_client_tokens_exceeded? + # Using Enumerable#sort_by on a Hash will typecast it into an associative + # Array (i.e. an Array of key-value Array pairs). However, since Hashes + # have an internal order in Ruby 1.9+, the resulting sorted associative + # Array can be converted back into a Hash, while maintaining the sorted + # order. + self.tokens = tokens.sort_by { |_cid, v| v[:expiry] || v['expiry'] }.to_h + + # Since the tokens are sorted by expiry, shift the oldest client token + # off the Hash until it no longer exceeds the maximum number of clients + tokens.shift while max_client_tokens_exceeded? end end end