From c7210791a625f7501f3eb863e4e9a83931ef0742 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Sun, 13 Nov 2022 17:16:30 +0000 Subject: [PATCH 01/48] PoC for counter-only rate limit class This proof of concept only sets a reset_after after the counter has already been exceeded. Next up, extending this so the reset_after is set the moment the counter reaches one (the first request after a reset) --- xbox/webapi/common/rate_limits.py | 62 +++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 xbox/webapi/common/rate_limits.py diff --git a/xbox/webapi/common/rate_limits.py b/xbox/webapi/common/rate_limits.py new file mode 100644 index 0000000..35c8114 --- /dev/null +++ b/xbox/webapi/common/rate_limits.py @@ -0,0 +1,62 @@ +from enum import Enum +from datetime import datetime, timedelta +from typing import Union +from pydantic import BaseModel + + +class TimePeriod(Enum): + BURST = 15 + SUSTAIN = 300 # 5 minutes (300s) + + +class LimitType(Enum): + WRITE = 0 + READ = 1 + + +class IncrementResult(BaseModel): + counter: int + exceeded: bool + + +class RateLimit: + def __init__(self, time_period: TimePeriod, limit: int): + self.__time_period = time_period + self.__limit = limit + + self.__exceeded: bool = False + self.__counter = 0 + # No requests so far, so reset_after is None. + self.__reset_after: Union[datetime, None] = None + + def get_time_period(self) -> "TimePeriod": + return self.__time_period + + def get_reset_after(self) -> Union[datetime, None]: + return self.__reset_after + + def is_exceeded(self) -> bool: + self.__reset_counter_if_required() + return self.__exceeded + + def increment(self) -> IncrementResult: + self.__reset_counter_if_required() + self.__counter += 1 + self.__check_if_exceeded() + return IncrementResult(counter=self.__counter, exceeded=self.__exceeded) + + # Should be called after every inc of the counter + def __check_if_exceeded(self): + if not self.__exceeded: + if self.__counter >= self.__limit: + self.__exceeded = True + self.__reset_after = datetime.now() + timedelta( + seconds=self.get_time_period().value + ) + + def __reset_counter_if_required(self): + if self.__reset_after is not None and self.__exceeded: + if self.__reset_after < datetime.now(): + self.__exceeded = False + self.__counter = 0 + self.__reset_after = None From df5daac7630e3a5f54270a842b9dae8d9ce00c6e Mon Sep 17 00:00:00 2001 From: James Cahill Date: Sun, 13 Nov 2022 17:55:45 +0000 Subject: [PATCH 02/48] Set reset_after on the first increment This commit changes the behaviour of the reset_after variable. Previously this variable would only be set when the limit was exceeded. The variable is now set on the first increment of a new cycle. (A cycle starting at instance creation or counter reset) This is more in line with how Xbox Live measures requests for rate limiting. * Set reset_after on the first increment * Create __set_reset_after function * Add code comments --- xbox/webapi/common/rate_limits.py | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/xbox/webapi/common/rate_limits.py b/xbox/webapi/common/rate_limits.py index 35c8114..3c12503 100644 --- a/xbox/webapi/common/rate_limits.py +++ b/xbox/webapi/common/rate_limits.py @@ -40,9 +40,20 @@ def is_exceeded(self) -> bool: return self.__exceeded def increment(self) -> IncrementResult: + # Call a function to check if the counter should be reset self.__reset_counter_if_required() + + # Increment the counter self.__counter += 1 + + # If the counter is 1, (first request after a reset) set the reset_after value. + if self.__counter == 1: + self.__set_reset_after() + + # Check to see if we have now exceeded the request limit self.__check_if_exceeded() + + # Return an instance of IncrementResult return IncrementResult(counter=self.__counter, exceeded=self.__exceeded) # Should be called after every inc of the counter @@ -50,13 +61,20 @@ def __check_if_exceeded(self): if not self.__exceeded: if self.__counter >= self.__limit: self.__exceeded = True - self.__reset_after = datetime.now() + timedelta( - seconds=self.get_time_period().value - ) + # reset-after is now dependent on the time since the first request of this cycle. + # self.__set_reset_after() def __reset_counter_if_required(self): - if self.__reset_after is not None and self.__exceeded: + # Check to make sure reset_after is not None + # - This is the case if this function is called before the counter + # is incremented after a reset / new instantiation + if self.__reset_after is not None: if self.__reset_after < datetime.now(): self.__exceeded = False self.__counter = 0 self.__reset_after = None + + def __set_reset_after(self): + self.__reset_after = datetime.now() + timedelta( + seconds=self.get_time_period().value + ) From 15967b825a821c1c34d842314bb95226d1221572 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Sun, 13 Nov 2022 17:59:23 +0000 Subject: [PATCH 03/48] Add LimitType param to init, add getter --- xbox/webapi/common/rate_limits.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/xbox/webapi/common/rate_limits.py b/xbox/webapi/common/rate_limits.py index 3c12503..df456ec 100644 --- a/xbox/webapi/common/rate_limits.py +++ b/xbox/webapi/common/rate_limits.py @@ -20,8 +20,9 @@ class IncrementResult(BaseModel): class RateLimit: - def __init__(self, time_period: TimePeriod, limit: int): + def __init__(self, time_period: TimePeriod, type: LimitType, limit: int): self.__time_period = time_period + self.__type = type self.__limit = limit self.__exceeded: bool = False @@ -31,6 +32,9 @@ def __init__(self, time_period: TimePeriod, limit: int): def get_time_period(self) -> "TimePeriod": return self.__time_period + + def get_limit_type(self) -> "LimitType": + return self.__type def get_reset_after(self) -> Union[datetime, None]: return self.__reset_after From 89667c2c0de38e18a9caa9eb7262e7c736a36cb6 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Sun, 13 Nov 2022 18:05:37 +0000 Subject: [PATCH 04/48] Split into '__init__' and 'models' files --- .../__init__.py} | 19 ++----------------- xbox/webapi/common/ratelimits/models.py | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 17 deletions(-) rename xbox/webapi/common/{rate_limits.py => ratelimits/__init__.py} (90%) create mode 100644 xbox/webapi/common/ratelimits/models.py diff --git a/xbox/webapi/common/rate_limits.py b/xbox/webapi/common/ratelimits/__init__.py similarity index 90% rename from xbox/webapi/common/rate_limits.py rename to xbox/webapi/common/ratelimits/__init__.py index df456ec..1593d5c 100644 --- a/xbox/webapi/common/rate_limits.py +++ b/xbox/webapi/common/ratelimits/__init__.py @@ -1,22 +1,7 @@ -from enum import Enum from datetime import datetime, timedelta from typing import Union -from pydantic import BaseModel - -class TimePeriod(Enum): - BURST = 15 - SUSTAIN = 300 # 5 minutes (300s) - - -class LimitType(Enum): - WRITE = 0 - READ = 1 - - -class IncrementResult(BaseModel): - counter: int - exceeded: bool +from xbox.webapi.common.ratelimits.models import TimePeriod, LimitType, IncrementResult class RateLimit: @@ -32,7 +17,7 @@ def __init__(self, time_period: TimePeriod, type: LimitType, limit: int): def get_time_period(self) -> "TimePeriod": return self.__time_period - + def get_limit_type(self) -> "LimitType": return self.__type diff --git a/xbox/webapi/common/ratelimits/models.py b/xbox/webapi/common/ratelimits/models.py new file mode 100644 index 0000000..18d9f6e --- /dev/null +++ b/xbox/webapi/common/ratelimits/models.py @@ -0,0 +1,17 @@ +from enum import Enum +from pydantic import BaseModel + + +class IncrementResult(BaseModel): + counter: int + exceeded: bool + + +class TimePeriod(Enum): + BURST = 15 # 15 seconds + SUSTAIN = 300 # 5 minutes (300s) + + +class LimitType(Enum): + WRITE = 0 + READ = 1 From 8462c9248271aa06990bd6857f903886060ad4c0 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Sun, 13 Nov 2022 18:17:46 +0000 Subject: [PATCH 05/48] Rename RateLimit to SingleRateLimit to prep for abstract class --- xbox/webapi/common/ratelimits/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xbox/webapi/common/ratelimits/__init__.py b/xbox/webapi/common/ratelimits/__init__.py index 1593d5c..a012ecf 100644 --- a/xbox/webapi/common/ratelimits/__init__.py +++ b/xbox/webapi/common/ratelimits/__init__.py @@ -4,7 +4,7 @@ from xbox.webapi.common.ratelimits.models import TimePeriod, LimitType, IncrementResult -class RateLimit: +class SingleRateLimit: def __init__(self, time_period: TimePeriod, type: LimitType, limit: int): self.__time_period = time_period self.__type = type From a9c2bf416dd84014263babc40f3a95fc78b66fac Mon Sep 17 00:00:00 2001 From: James Cahill Date: Sun, 13 Nov 2022 18:22:38 +0000 Subject: [PATCH 06/48] Create initial abtract base class --- xbox/webapi/common/ratelimits/__init__.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/xbox/webapi/common/ratelimits/__init__.py b/xbox/webapi/common/ratelimits/__init__.py index a012ecf..f21437d 100644 --- a/xbox/webapi/common/ratelimits/__init__.py +++ b/xbox/webapi/common/ratelimits/__init__.py @@ -3,8 +3,26 @@ from xbox.webapi.common.ratelimits.models import TimePeriod, LimitType, IncrementResult +from abc import ABCMeta, abstractmethod -class SingleRateLimit: + +class RateLimit(metaclass=ABCMeta): + @abstractmethod + def get_time_period(self) -> "TimePeriod": + pass + + @abstractmethod + def get_reset_after(self) -> Union[datetime, None]: + pass + + def is_exceeded(self) -> bool: + pass + + def increment(self) -> IncrementResult: + pass + + +class SingleRateLimit(RateLimit): def __init__(self, time_period: TimePeriod, type: LimitType, limit: int): self.__time_period = time_period self.__type = type From c96bab45a7f01e22b5298ed0dd5fad0b028c961d Mon Sep 17 00:00:00 2001 From: James Cahill Date: Sun, 13 Nov 2022 18:23:50 +0000 Subject: [PATCH 07/48] Revert "Create initial abtract base class" This reverts commit a9c2bf416dd84014263babc40f3a95fc78b66fac. --- xbox/webapi/common/ratelimits/__init__.py | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/xbox/webapi/common/ratelimits/__init__.py b/xbox/webapi/common/ratelimits/__init__.py index f21437d..a012ecf 100644 --- a/xbox/webapi/common/ratelimits/__init__.py +++ b/xbox/webapi/common/ratelimits/__init__.py @@ -3,26 +3,8 @@ from xbox.webapi.common.ratelimits.models import TimePeriod, LimitType, IncrementResult -from abc import ABCMeta, abstractmethod - -class RateLimit(metaclass=ABCMeta): - @abstractmethod - def get_time_period(self) -> "TimePeriod": - pass - - @abstractmethod - def get_reset_after(self) -> Union[datetime, None]: - pass - - def is_exceeded(self) -> bool: - pass - - def increment(self) -> IncrementResult: - pass - - -class SingleRateLimit(RateLimit): +class SingleRateLimit: def __init__(self, time_period: TimePeriod, type: LimitType, limit: int): self.__time_period = time_period self.__type = type From 45adcc49d6a4b0bf5e6ede7124624275af240b6a Mon Sep 17 00:00:00 2001 From: James Cahill Date: Sun, 13 Nov 2022 18:24:37 +0000 Subject: [PATCH 08/48] Revert "Rename RateLimit to SingleRateLimit to prep for abstract class" This reverts commit 8462c9248271aa06990bd6857f903886060ad4c0. --- xbox/webapi/common/ratelimits/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xbox/webapi/common/ratelimits/__init__.py b/xbox/webapi/common/ratelimits/__init__.py index a012ecf..1593d5c 100644 --- a/xbox/webapi/common/ratelimits/__init__.py +++ b/xbox/webapi/common/ratelimits/__init__.py @@ -4,7 +4,7 @@ from xbox.webapi.common.ratelimits.models import TimePeriod, LimitType, IncrementResult -class SingleRateLimit: +class RateLimit: def __init__(self, time_period: TimePeriod, type: LimitType, limit: int): self.__time_period = time_period self.__type = type From a810b0a36b52da58bf612d807f2d9869b252ae7f Mon Sep 17 00:00:00 2001 From: James Cahill Date: Mon, 14 Nov 2022 07:31:44 +0000 Subject: [PATCH 09/48] Add ParsedRateLimit model --- xbox/webapi/common/ratelimits/models.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/xbox/webapi/common/ratelimits/models.py b/xbox/webapi/common/ratelimits/models.py index 18d9f6e..d8bd212 100644 --- a/xbox/webapi/common/ratelimits/models.py +++ b/xbox/webapi/common/ratelimits/models.py @@ -7,6 +7,11 @@ class IncrementResult(BaseModel): exceeded: bool +class ParsedRateLimit(BaseModel): + read: int + write: int + + class TimePeriod(Enum): BURST = 15 # 15 seconds SUSTAIN = 300 # 5 minutes (300s) From 0997e8e58a5b8da300b285de6effc7aef6bf9eaa Mon Sep 17 00:00:00 2001 From: James Cahill Date: Mon, 14 Nov 2022 07:33:33 +0000 Subject: [PATCH 10/48] PoC ratelimitedprovider with RATE_LIMIT dict parsing --- .../api/provider/ratelimitedprovider.py | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 xbox/webapi/api/provider/ratelimitedprovider.py diff --git a/xbox/webapi/api/provider/ratelimitedprovider.py b/xbox/webapi/api/provider/ratelimitedprovider.py new file mode 100644 index 0000000..e60d058 --- /dev/null +++ b/xbox/webapi/api/provider/ratelimitedprovider.py @@ -0,0 +1,45 @@ +""" +RateLimitedProvider + +Subclassed by providers with rate limit support +""" + +from typing import Union +from xbox.webapi.api.provider.baseprovider import BaseProvider +from xbox.webapi.common.ratelimits.models import ParsedRateLimit + + +class RateLimitedProvider(BaseProvider): + RATE_LIMITS: dict[str, Union[int, dict[str, int]]] + + def __init__(self, client): + """ + Initialize Baseclass + + Args: + client (:class:`XboxLiveClient`): Instance of XboxLiveClient + """ + # [commented out for testing] super().__init__(client) + print(self.RATE_LIMITS) + + # Parse the rate limit dict + burst_key = self.RATE_LIMITS["burst"] + sustain_key = self.RATE_LIMITS["sustain"] + + burst_rate_limits = self.__parse_rate_limit_key(burst_key) + sustain_rate_liits = self.__parse_rate_limit_key(sustain_key) + + print(burst_rate_limits) + print(sustain_rate_liits) + + def __parse_rate_limit_key( + self, key: Union[int, dict[str, int]] + ) -> ParsedRateLimit: + key_type = type(key) + if key_type == int: + return ParsedRateLimit(read=key, write=key) + elif key_type == dict: + # TODO: schema here? + # Since the key-value pairs match we can just pass the dict to the model + return ParsedRateLimit(**key) + # return ParsedRateLimit(read=key["read"], write=key["write"]) From a8bb0d0f6233406b8c38fb061c8f62db3a6dfa8a Mon Sep 17 00:00:00 2001 From: James Cahill Date: Mon, 14 Nov 2022 07:34:10 +0000 Subject: [PATCH 11/48] Use RateLimitedProvider as base class for PeopleProvider --- xbox/webapi/api/provider/people/__init__.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/xbox/webapi/api/provider/people/__init__.py b/xbox/webapi/api/provider/people/__init__.py index 2c1ca7c..8daed99 100644 --- a/xbox/webapi/api/provider/people/__init__.py +++ b/xbox/webapi/api/provider/people/__init__.py @@ -3,7 +3,7 @@ """ from typing import List -from xbox.webapi.api.provider.baseprovider import BaseProvider +from xbox.webapi.api.provider.ratelimitedprovider import RateLimitedProvider from xbox.webapi.api.provider.people.models import ( PeopleDecoration, PeopleResponse, @@ -11,7 +11,7 @@ ) -class PeopleProvider(BaseProvider): +class PeopleProvider(RateLimitedProvider): SOCIAL_URL = "https://social.xboxlive.com" HEADERS_SOCIAL = {"x-xbl-contract-version": "2"} PEOPLE_URL = "https://peoplehub.xboxlive.com" @@ -21,6 +21,8 @@ class PeopleProvider(BaseProvider): } SEPERATOR = "," + RATE_LIMITS = {"BURST": 10, "SUSTAIN": 30} + def __init__(self, client): """ Initialize Baseclass, set 'Accept-Language' header from client instance From e1b37310697a59e8de75df166f911703d588508b Mon Sep 17 00:00:00 2001 From: James Cahill Date: Sun, 13 Nov 2022 18:17:46 +0000 Subject: [PATCH 12/48] Rename RateLimit to SingleRateLimit to prep for abstract class --- xbox/webapi/common/ratelimits/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xbox/webapi/common/ratelimits/__init__.py b/xbox/webapi/common/ratelimits/__init__.py index 1593d5c..a012ecf 100644 --- a/xbox/webapi/common/ratelimits/__init__.py +++ b/xbox/webapi/common/ratelimits/__init__.py @@ -4,7 +4,7 @@ from xbox.webapi.common.ratelimits.models import TimePeriod, LimitType, IncrementResult -class RateLimit: +class SingleRateLimit: def __init__(self, time_period: TimePeriod, type: LimitType, limit: int): self.__time_period = time_period self.__type = type From 1c8f48e7c2a6aa6633ab242d4b04159a27af4ef1 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Sun, 13 Nov 2022 18:22:38 +0000 Subject: [PATCH 13/48] Create initial abtract base class --- xbox/webapi/common/ratelimits/__init__.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/xbox/webapi/common/ratelimits/__init__.py b/xbox/webapi/common/ratelimits/__init__.py index a012ecf..f21437d 100644 --- a/xbox/webapi/common/ratelimits/__init__.py +++ b/xbox/webapi/common/ratelimits/__init__.py @@ -3,8 +3,26 @@ from xbox.webapi.common.ratelimits.models import TimePeriod, LimitType, IncrementResult +from abc import ABCMeta, abstractmethod -class SingleRateLimit: + +class RateLimit(metaclass=ABCMeta): + @abstractmethod + def get_time_period(self) -> "TimePeriod": + pass + + @abstractmethod + def get_reset_after(self) -> Union[datetime, None]: + pass + + def is_exceeded(self) -> bool: + pass + + def increment(self) -> IncrementResult: + pass + + +class SingleRateLimit(RateLimit): def __init__(self, time_period: TimePeriod, type: LimitType, limit: int): self.__time_period = time_period self.__type = type From 085f617a2faed9306d89679da819a86aa55bddec Mon Sep 17 00:00:00 2001 From: James Cahill Date: Mon, 14 Nov 2022 07:56:30 +0000 Subject: [PATCH 14/48] Add time period variable to ParsedRateLimit model --- xbox/webapi/common/ratelimits/models.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/xbox/webapi/common/ratelimits/models.py b/xbox/webapi/common/ratelimits/models.py index d8bd212..cde7612 100644 --- a/xbox/webapi/common/ratelimits/models.py +++ b/xbox/webapi/common/ratelimits/models.py @@ -2,16 +2,6 @@ from pydantic import BaseModel -class IncrementResult(BaseModel): - counter: int - exceeded: bool - - -class ParsedRateLimit(BaseModel): - read: int - write: int - - class TimePeriod(Enum): BURST = 15 # 15 seconds SUSTAIN = 300 # 5 minutes (300s) @@ -20,3 +10,14 @@ class TimePeriod(Enum): class LimitType(Enum): WRITE = 0 READ = 1 + + +class IncrementResult(BaseModel): + counter: int + exceeded: bool + + +class ParsedRateLimit(BaseModel): + read: int + write: int + period: TimePeriod From 09147ea2445a7f7316d44167638a8b1d5012a21e Mon Sep 17 00:00:00 2001 From: James Cahill Date: Mon, 14 Nov 2022 07:59:58 +0000 Subject: [PATCH 15/48] pass period key when instanciating ParsedRateLimit --- xbox/webapi/api/provider/ratelimitedprovider.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/xbox/webapi/api/provider/ratelimitedprovider.py b/xbox/webapi/api/provider/ratelimitedprovider.py index e60d058..fa7d9b7 100644 --- a/xbox/webapi/api/provider/ratelimitedprovider.py +++ b/xbox/webapi/api/provider/ratelimitedprovider.py @@ -6,7 +6,7 @@ from typing import Union from xbox.webapi.api.provider.baseprovider import BaseProvider -from xbox.webapi.common.ratelimits.models import ParsedRateLimit +from xbox.webapi.common.ratelimits.models import ParsedRateLimit, TimePeriod class RateLimitedProvider(BaseProvider): @@ -26,20 +26,22 @@ def __init__(self, client): burst_key = self.RATE_LIMITS["burst"] sustain_key = self.RATE_LIMITS["sustain"] - burst_rate_limits = self.__parse_rate_limit_key(burst_key) - sustain_rate_liits = self.__parse_rate_limit_key(sustain_key) + burst_rate_limits = self.__parse_rate_limit_key(burst_key, TimePeriod.BURST) + sustain_rate_liits = self.__parse_rate_limit_key( + sustain_key, TimePeriod.SUSTAIN + ) print(burst_rate_limits) print(sustain_rate_liits) def __parse_rate_limit_key( - self, key: Union[int, dict[str, int]] + self, key: Union[int, dict[str, int]], period: TimePeriod ) -> ParsedRateLimit: key_type = type(key) if key_type == int: - return ParsedRateLimit(read=key, write=key) + return ParsedRateLimit(read=key, write=key, period=period) elif key_type == dict: # TODO: schema here? # Since the key-value pairs match we can just pass the dict to the model - return ParsedRateLimit(**key) + return ParsedRateLimit(**key, period=period) # return ParsedRateLimit(read=key["read"], write=key["write"]) From 7089acaf1365410cd082baa1eaa9d90632d79101 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Mon, 14 Nov 2022 08:38:28 +0000 Subject: [PATCH 16/48] update RATE_LIMITS dict keys --- xbox/webapi/api/provider/people/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xbox/webapi/api/provider/people/__init__.py b/xbox/webapi/api/provider/people/__init__.py index 8daed99..a96e2e1 100644 --- a/xbox/webapi/api/provider/people/__init__.py +++ b/xbox/webapi/api/provider/people/__init__.py @@ -21,7 +21,7 @@ class PeopleProvider(RateLimitedProvider): } SEPERATOR = "," - RATE_LIMITS = {"BURST": 10, "SUSTAIN": 30} + RATE_LIMITS = {"burst": 10, "sustain": 30} def __init__(self, client): """ From 8d61d1c6d1aff9e249589c5359e31c3ee23bde3d Mon Sep 17 00:00:00 2001 From: James Cahill Date: Mon, 14 Nov 2022 08:44:05 +0000 Subject: [PATCH 17/48] Initial __init__ for CombinedRateLimit The init takes *ParsedRateLimit as paramaters and constructs SingleRateLimit objects for them. --- xbox/webapi/common/ratelimits/__init__.py | 29 ++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/xbox/webapi/common/ratelimits/__init__.py b/xbox/webapi/common/ratelimits/__init__.py index f21437d..048f2a5 100644 --- a/xbox/webapi/common/ratelimits/__init__.py +++ b/xbox/webapi/common/ratelimits/__init__.py @@ -1,7 +1,12 @@ from datetime import datetime, timedelta from typing import Union -from xbox.webapi.common.ratelimits.models import TimePeriod, LimitType, IncrementResult +from xbox.webapi.common.ratelimits.models import ( + ParsedRateLimit, + TimePeriod, + LimitType, + IncrementResult, +) from abc import ABCMeta, abstractmethod @@ -85,3 +90,25 @@ def __set_reset_after(self): self.__reset_after = datetime.now() + timedelta( seconds=self.get_time_period().value ) + + +class CombinedRateLimit: + def __init__(self, *parsed_limits: ParsedRateLimit, type: LimitType): + # *parsed_limits is a tuple + + # Create a SingleRateLimit instance for each limit + self.__limits: list[SingleRateLimit] = [] + + for limit in parsed_limits: + limit_num = limit.read if type == LimitType.READ else limit.write + print( + "Found %s limit of %i (%s)" % (limit.period.name, limit_num, type.name) + ) + self.__limits.append(SingleRateLimit(limit.period, type, limit_num)) + + # DEBUG: print them + for i in self.__limits: + print( + "Added limit of type %s, limit %s, and limit %i" + % (i.get_limit_type(), i.get_time_period(), i._SingleRateLimit__limit) + ) From 103ef0f4449134bd6bd844c97b4636840857138b Mon Sep 17 00:00:00 2001 From: James Cahill Date: Mon, 14 Nov 2022 08:45:23 +0000 Subject: [PATCH 18/48] Call super init, instanciate CombinedRateLimits, fix spelling in variable name --- xbox/webapi/api/provider/ratelimitedprovider.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/xbox/webapi/api/provider/ratelimitedprovider.py b/xbox/webapi/api/provider/ratelimitedprovider.py index fa7d9b7..00b0833 100644 --- a/xbox/webapi/api/provider/ratelimitedprovider.py +++ b/xbox/webapi/api/provider/ratelimitedprovider.py @@ -6,7 +6,8 @@ from typing import Union from xbox.webapi.api.provider.baseprovider import BaseProvider -from xbox.webapi.common.ratelimits.models import ParsedRateLimit, TimePeriod +from xbox.webapi.common.ratelimits.models import LimitType, ParsedRateLimit, TimePeriod +from xbox.webapi.common.ratelimits import CombinedRateLimit class RateLimitedProvider(BaseProvider): @@ -19,20 +20,21 @@ def __init__(self, client): Args: client (:class:`XboxLiveClient`): Instance of XboxLiveClient """ - # [commented out for testing] super().__init__(client) - print(self.RATE_LIMITS) + super().__init__(client) - # Parse the rate limit dict + # Retrieve burst and sustain from the dict burst_key = self.RATE_LIMITS["burst"] sustain_key = self.RATE_LIMITS["sustain"] + # Parse the rate limit dict values burst_rate_limits = self.__parse_rate_limit_key(burst_key, TimePeriod.BURST) - sustain_rate_liits = self.__parse_rate_limit_key( + sustain_rate_limits = self.__parse_rate_limit_key( sustain_key, TimePeriod.SUSTAIN ) - print(burst_rate_limits) - print(sustain_rate_liits) + # Instanciate CombinedRateLimits for read and write respectively + CombinedRateLimit(burst_rate_limits, sustain_rate_limits, type=LimitType.READ) + CombinedRateLimit(burst_rate_limits, sustain_rate_limits, type=LimitType.WRITE) def __parse_rate_limit_key( self, key: Union[int, dict[str, int]], period: TimePeriod From c01c330dde284a80596c9e40839af2fe84a9c5dc Mon Sep 17 00:00:00 2001 From: James Cahill Date: Mon, 14 Nov 2022 08:47:15 +0000 Subject: [PATCH 19/48] Clean up init, remove duplicate debug print --- xbox/webapi/common/ratelimits/__init__.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/xbox/webapi/common/ratelimits/__init__.py b/xbox/webapi/common/ratelimits/__init__.py index 048f2a5..0fd3061 100644 --- a/xbox/webapi/common/ratelimits/__init__.py +++ b/xbox/webapi/common/ratelimits/__init__.py @@ -100,11 +100,12 @@ def __init__(self, *parsed_limits: ParsedRateLimit, type: LimitType): self.__limits: list[SingleRateLimit] = [] for limit in parsed_limits: + # Use the type param (enum LimitType) to determine which limit to select limit_num = limit.read if type == LimitType.READ else limit.write - print( - "Found %s limit of %i (%s)" % (limit.period.name, limit_num, type.name) - ) - self.__limits.append(SingleRateLimit(limit.period, type, limit_num)) + + # Create a new instance of SingleRateLimit and append it to the limits array. + srl = SingleRateLimit(limit.period, type, limit_num) + self.__limits.append(srl) # DEBUG: print them for i in self.__limits: From d240b2b3b446f885886b0740e103284bb5fed988 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Mon, 14 Nov 2022 10:55:25 +0000 Subject: [PATCH 20/48] (CombinedRateLimit): Create .get_reset_after and .is_exceeded funcs --- xbox/webapi/common/ratelimits/__init__.py | 41 +++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/xbox/webapi/common/ratelimits/__init__.py b/xbox/webapi/common/ratelimits/__init__.py index 0fd3061..03d58e0 100644 --- a/xbox/webapi/common/ratelimits/__init__.py +++ b/xbox/webapi/common/ratelimits/__init__.py @@ -113,3 +113,44 @@ def __init__(self, *parsed_limits: ParsedRateLimit, type: LimitType): "Added limit of type %s, limit %s, and limit %i" % (i.get_limit_type(), i.get_time_period(), i._SingleRateLimit__limit) ) + + def get_reset_after(self) -> Union[datetime, None]: + # Map self.__limits to (limit).get_reset_after() + dates_map = map(lambda limit: limit.get_reset_after(), self.__limits) + + # Convert the map object to a list + dates = list(dates_map) + + # Construct a new list with only elements of instance datetime + # (Effectively filtering out any None elements) + dates_valid = [elem for elem in dates if type(elem) == datetime] + + # If dates_valid has any elements, return the one with the *later* timestamp. + if len(dates_valid) != 0: + dates_valid[0].isoformat + print( + "Valid dates BEFORE sorting: %s" + % list(map(lambda i: i.isoformat(), dates_valid)) + ) + # By default dates are sorted with the earliest date first. + # We will set reverse=True so that the first element is the later date. + dates_valid.sort(reverse=True) + + print( + "Valid dates AFTER sorting: %s" + % list(map(lambda i: i.isoformat(), dates_valid)) + ) + + # Return the datetime object. + return dates_valid[0] + + # dates_valid has no elements, return None + return None + + def is_exceeded(self) -> bool: + # Map self.__limits to (limit).is_exceeded() + is_exceeded_map = map(lambda limit: limit.is_exceeded(), self.__limits) + is_exceeded_list = list(is_exceeded_map) + + # Return True if any variable in list is True + return True in is_exceeded_list From 0bce303f38cbd8c1c9225f34f1597edb881273e2 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Mon, 14 Nov 2022 11:04:52 +0000 Subject: [PATCH 21/48] (CombinedRateLimit): Create .increment func --- xbox/webapi/common/ratelimits/__init__.py | 24 +++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/xbox/webapi/common/ratelimits/__init__.py b/xbox/webapi/common/ratelimits/__init__.py index 03d58e0..9f7d650 100644 --- a/xbox/webapi/common/ratelimits/__init__.py +++ b/xbox/webapi/common/ratelimits/__init__.py @@ -154,3 +154,27 @@ def is_exceeded(self) -> bool: # Return True if any variable in list is True return True in is_exceeded_list + + def increment(self) -> IncrementResult: + # Increment each limit + results: list[IncrementResult] = [] + for limit in self.__limits: + result = limit.increment() + results.append(result) + + # SPEC: Which counter should be picked here? + # For now, let's pick the *higher* counter + # (should incrementResult even include the counter?) + results[1].counter = 5 + + # By default, sorted() returns in ascending order, so let's set reverse=True + # This means that the result with the highest counter will be the first element. + results_sorted = sorted(results, key=lambda i: i.counter, reverse=True) + + # Create an instance of IncrementResult and return it. + return IncrementResult( + counter=results_sorted[ + 0 + ].counter, # Use the highest counter (sorted in descending order) + exceeded=self.is_exceeded(), # Call self.is_exceeded (True if any limit has been exceeded, a-la an OR gate.) + ) From 2cc4cd41afbe2b37764b206e4e273aebf0c38850 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Mon, 14 Nov 2022 11:06:15 +0000 Subject: [PATCH 22/48] Add abstractmethod decorator to RateLimit funcs --- xbox/webapi/common/ratelimits/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/xbox/webapi/common/ratelimits/__init__.py b/xbox/webapi/common/ratelimits/__init__.py index 9f7d650..cb6f14a 100644 --- a/xbox/webapi/common/ratelimits/__init__.py +++ b/xbox/webapi/common/ratelimits/__init__.py @@ -20,9 +20,11 @@ def get_time_period(self) -> "TimePeriod": def get_reset_after(self) -> Union[datetime, None]: pass + @abstractmethod def is_exceeded(self) -> bool: pass + @abstractmethod def increment(self) -> IncrementResult: pass From 85c34495257ff4f9cc50380b2f83bbd17e6bac8a Mon Sep 17 00:00:00 2001 From: James Cahill Date: Mon, 14 Nov 2022 11:06:50 +0000 Subject: [PATCH 23/48] Remove get_time_period from RateLimit This function is not currently implemented in CombinedRateLimit. --- xbox/webapi/common/ratelimits/__init__.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/xbox/webapi/common/ratelimits/__init__.py b/xbox/webapi/common/ratelimits/__init__.py index cb6f14a..d7bd621 100644 --- a/xbox/webapi/common/ratelimits/__init__.py +++ b/xbox/webapi/common/ratelimits/__init__.py @@ -12,10 +12,6 @@ class RateLimit(metaclass=ABCMeta): - @abstractmethod - def get_time_period(self) -> "TimePeriod": - pass - @abstractmethod def get_reset_after(self) -> Union[datetime, None]: pass From 9249f6f86d43cac2dc72ec410200f590a3045826 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Mon, 14 Nov 2022 11:08:07 +0000 Subject: [PATCH 24/48] (RateLimitedProvider): Assign CombinedRateLimit instances to class variables --- xbox/webapi/api/provider/ratelimitedprovider.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/xbox/webapi/api/provider/ratelimitedprovider.py b/xbox/webapi/api/provider/ratelimitedprovider.py index 00b0833..5eea1cc 100644 --- a/xbox/webapi/api/provider/ratelimitedprovider.py +++ b/xbox/webapi/api/provider/ratelimitedprovider.py @@ -33,8 +33,12 @@ def __init__(self, client): ) # Instanciate CombinedRateLimits for read and write respectively - CombinedRateLimit(burst_rate_limits, sustain_rate_limits, type=LimitType.READ) - CombinedRateLimit(burst_rate_limits, sustain_rate_limits, type=LimitType.WRITE) + self.rate_limit_read = CombinedRateLimit( + burst_rate_limits, sustain_rate_limits, type=LimitType.READ + ) + self.rate_limit_write = CombinedRateLimit( + burst_rate_limits, sustain_rate_limits, type=LimitType.WRITE + ) def __parse_rate_limit_key( self, key: Union[int, dict[str, int]], period: TimePeriod From 8b46dd88c5dee53de31ec948037144da7aaf23f0 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Mon, 14 Nov 2022 11:44:42 +0000 Subject: [PATCH 25/48] Create RateLimitExceededException --- xbox/webapi/common/exceptions.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/xbox/webapi/common/exceptions.py b/xbox/webapi/common/exceptions.py index 44da117..0e278e5 100644 --- a/xbox/webapi/common/exceptions.py +++ b/xbox/webapi/common/exceptions.py @@ -3,6 +3,9 @@ """ +from xbox.webapi.common.ratelimits import RateLimit + + class XboxException(Exception): """Base exception for all Xbox exceptions to subclass""" @@ -46,3 +49,10 @@ class NotFoundException(XboxException): """Any exception raised due to a resource being missing will subclass this""" pass + + +class RateLimitExceededException(XboxException): + def __init__(self, message, rate_limit: RateLimit): + self.message = message + self.rate_limit = rate_limit + self.try_again_in = rate_limit.get_reset_after() From 5f836c75f77fffd2edae6e453f3b4abd78b3bfd8 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Mon, 14 Nov 2022 11:46:46 +0000 Subject: [PATCH 26/48] Allow optional passing of rate_limit in kwargs * Checks if the rate limit is exceeded, throws exception if True * TODO: Change this to a *middleware* that also increments the counter after a request --- xbox/webapi/api/client.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/xbox/webapi/api/client.py b/xbox/webapi/api/client.py index 6cf17ec..71fa472 100644 --- a/xbox/webapi/api/client.py +++ b/xbox/webapi/api/client.py @@ -28,6 +28,8 @@ from xbox.webapi.api.provider.usersearch import UserSearchProvider from xbox.webapi.api.provider.userstats import UserStatsProvider from xbox.webapi.authentication.manager import AuthenticationManager +from xbox.webapi.common.exceptions import RateLimitExceededException +from xbox.webapi.common.ratelimits import RateLimit log = logging.getLogger("xbox.api") @@ -55,6 +57,9 @@ async def request( extra_params = kwargs.pop("extra_params", None) extra_data = kwargs.pop("extra_data", None) + # Rate limit object + rate_limits: RateLimit = kwargs.pop("rate_limits", None) + if include_auth: # Ensure tokens valid await self._auth_mgr.refresh_tokens() @@ -78,6 +83,11 @@ async def request( data = data or {} data.update(extra_data) + if rate_limits: + # Check if rate limits have been exceeded for this endpoint + if rate_limits.is_exceeded(): + raise RateLimitExceededException("Rate limit exceeded", rate_limits) + return await self._auth_mgr.session.request( method, url, **kwargs, headers=headers, params=params, data=data ) From 4f7e30b1efc52270f75d7f68bdb2612fa5874a82 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Mon, 14 Nov 2022 12:14:17 +0000 Subject: [PATCH 27/48] (RateLimit): Add get_counter abstract and child methods --- xbox/webapi/common/ratelimits/__init__.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/xbox/webapi/common/ratelimits/__init__.py b/xbox/webapi/common/ratelimits/__init__.py index d7bd621..0b26f2c 100644 --- a/xbox/webapi/common/ratelimits/__init__.py +++ b/xbox/webapi/common/ratelimits/__init__.py @@ -12,6 +12,10 @@ class RateLimit(metaclass=ABCMeta): + @abstractmethod + def get_counter(self) -> int: + pass + @abstractmethod def get_reset_after(self) -> Union[datetime, None]: pass @@ -36,6 +40,9 @@ def __init__(self, time_period: TimePeriod, type: LimitType, limit: int): # No requests so far, so reset_after is None. self.__reset_after: Union[datetime, None] = None + def get_counter(self) -> int: + return self.__counter + def get_time_period(self) -> "TimePeriod": return self.__time_period @@ -112,6 +119,18 @@ def __init__(self, *parsed_limits: ParsedRateLimit, type: LimitType): % (i.get_limit_type(), i.get_time_period(), i._SingleRateLimit__limit) ) + def get_counter(self) -> int: + # Map self.__limits to (limit).get_counter() + counter_map = map(lambda limit: limit.get_counter(), self.__limits) + counters = list(counter_map) + + # Sort the counters list by value + # reverse=True to get highest first + counters.sort(reverse=True) + + # Return the highest value + return counters[0] + def get_reset_after(self) -> Union[datetime, None]: # Map self.__limits to (limit).get_reset_after() dates_map = map(lambda limit: limit.get_reset_after(), self.__limits) From 2bc428a763dabee9d27e5562d8e601a41c71ea05 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Mon, 14 Nov 2022 12:26:56 +0000 Subject: [PATCH 28/48] (CombinedRateLimit.get_reset_after) filter out limits that have not been exceeded --- xbox/webapi/common/ratelimits/__init__.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/xbox/webapi/common/ratelimits/__init__.py b/xbox/webapi/common/ratelimits/__init__.py index 0b26f2c..50d903e 100644 --- a/xbox/webapi/common/ratelimits/__init__.py +++ b/xbox/webapi/common/ratelimits/__init__.py @@ -131,9 +131,14 @@ def get_counter(self) -> int: # Return the highest value return counters[0] + # We don't want a datetime response for a limit that has not been exceeded. + # Otherwise eg. 10 burst requests -> 300s timeout (should be 30 (burst exceeded), 300s (not exceeded) def get_reset_after(self) -> Union[datetime, None]: + # Get a list of limits that *have been exceeded* + dates_exceeded_only = filter(lambda limit: limit.is_exceeded(), self.__limits) + # Map self.__limits to (limit).get_reset_after() - dates_map = map(lambda limit: limit.get_reset_after(), self.__limits) + dates_map = map(lambda limit: limit.get_reset_after(), dates_exceeded_only) # Convert the map object to a list dates = list(dates_map) @@ -143,6 +148,7 @@ def get_reset_after(self) -> Union[datetime, None]: dates_valid = [elem for elem in dates if type(elem) == datetime] # If dates_valid has any elements, return the one with the *later* timestamp. + # This means that if two or more limits have been exceeded, we wait for both to have reset (by returning the later timestamp) if len(dates_valid) != 0: dates_valid[0].isoformat print( From 0c79b6a6613f4a8810e3be61f42c236253bfb311 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Mon, 14 Nov 2022 12:28:27 +0000 Subject: [PATCH 29/48] (client) wheen given, increment rate limit counter before returning response NOTE: This doesn't seem particularly efficient? two if statements --- xbox/webapi/api/client.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/xbox/webapi/api/client.py b/xbox/webapi/api/client.py index 71fa472..4276685 100644 --- a/xbox/webapi/api/client.py +++ b/xbox/webapi/api/client.py @@ -88,10 +88,15 @@ async def request( if rate_limits.is_exceeded(): raise RateLimitExceededException("Rate limit exceeded", rate_limits) - return await self._auth_mgr.session.request( + response = await self._auth_mgr.session.request( method, url, **kwargs, headers=headers, params=params, data=data ) + if rate_limits: + rate_limits.increment() + + return response + async def get(self, url: str, **kwargs: Any) -> Response: return await self.request("GET", url, **kwargs) From 8e4d11c6e31572fb9930b09aa72fcadb0d7f8247 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Mon, 14 Nov 2022 12:29:17 +0000 Subject: [PATCH 30/48] (get_friends_own): pass rate limits to client session for testing --- xbox/webapi/api/provider/people/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/xbox/webapi/api/provider/people/__init__.py b/xbox/webapi/api/provider/people/__init__.py index a96e2e1..72fd037 100644 --- a/xbox/webapi/api/provider/people/__init__.py +++ b/xbox/webapi/api/provider/people/__init__.py @@ -53,7 +53,9 @@ async def get_friends_own( decoration = self.SEPERATOR.join(decoration_fields) url = f"{self.PEOPLE_URL}/users/me/people/social/decoration/{decoration}" - resp = await self.client.session.get(url, headers=self._headers, **kwargs) + resp = await self.client.session.get( + url, headers=self._headers, rate_limits=self.rate_limit_read, **kwargs + ) resp.raise_for_status() return PeopleResponse(**resp.json()) From 41c641cb050be9269bfef2a38a875532e47e39e2 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Mon, 14 Nov 2022 12:30:25 +0000 Subject: [PATCH 31/48] Initial rate limit test --- tests/test_ratelimits.py | 44 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 tests/test_ratelimits.py diff --git a/tests/test_ratelimits.py b/tests/test_ratelimits.py new file mode 100644 index 0000000..cbe4070 --- /dev/null +++ b/tests/test_ratelimits.py @@ -0,0 +1,44 @@ +from datetime import datetime, timedelta +from httpx import Response +import pytest + +from tests.common import get_response_json + +from xbox.webapi.common.exceptions import RateLimitExceededException + + +@pytest.mark.asyncio +async def test_ratelimits_exceeded_burst_only(respx_mock, xbl_client): + async def make_request(): + route = respx_mock.get("https://peoplehub.xboxlive.com").mock( + return_value=Response(200, json=get_response_json("people_friends_own")) + ) + ret = await xbl_client.people.get_friends_own() + + assert len(ret.people) == 2 + assert route.called + + # Record the start time to ensure that the timeouts are the correct length + start_time = datetime.now() + + # Make as many requests as possible without exceeding + max_request_num = xbl_client.people.RATE_LIMITS["burst"] + for i in range(max_request_num): + await make_request() + + # Make another request, ensure that it raises the exception. + with pytest.raises(RateLimitExceededException) as exception: + await make_request() + + # Get the error instance from pytest + ex: RateLimitExceededException = exception.value + + # Assert that the counter matches the max request num (should not have incremented above max value) + assert ex.rate_limit.get_counter() == max_request_num + + # Get the timeout we were issued + try_again_in = ex.rate_limit.get_reset_after() + + # Assert that the timeout is the correct length + delta: timedelta = try_again_in - start_time + assert delta.seconds == 15 From f12d9f0e3b6c1ce215152692f9de611b0a78ce39 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Mon, 14 Nov 2022 13:32:50 +0000 Subject: [PATCH 32/48] Use typing.Dict for dict hints Hopefully fixes a TypeError on python>3.9 --- xbox/webapi/api/provider/ratelimitedprovider.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/xbox/webapi/api/provider/ratelimitedprovider.py b/xbox/webapi/api/provider/ratelimitedprovider.py index 5eea1cc..58dab1f 100644 --- a/xbox/webapi/api/provider/ratelimitedprovider.py +++ b/xbox/webapi/api/provider/ratelimitedprovider.py @@ -4,14 +4,15 @@ Subclassed by providers with rate limit support """ -from typing import Union +from typing import Union, Dict from xbox.webapi.api.provider.baseprovider import BaseProvider from xbox.webapi.common.ratelimits.models import LimitType, ParsedRateLimit, TimePeriod from xbox.webapi.common.ratelimits import CombinedRateLimit class RateLimitedProvider(BaseProvider): - RATE_LIMITS: dict[str, Union[int, dict[str, int]]] + # dict -> Dict (typing.dict) https://stackoverflow.com/a/63460173 + RATE_LIMITS: Dict[str, Union[int, Dict[str, int]]] def __init__(self, client): """ @@ -41,7 +42,7 @@ def __init__(self, client): ) def __parse_rate_limit_key( - self, key: Union[int, dict[str, int]], period: TimePeriod + self, key: Union[int, Dict[str, int]], period: TimePeriod ) -> ParsedRateLimit: key_type = type(key) if key_type == int: From a6eb66339aedb0112d8de46322f3f23ddeeb51b3 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Tue, 15 Nov 2022 11:34:16 +0000 Subject: [PATCH 33/48] Set RateLimit as parent class of CombinedRateLimit --- xbox/webapi/common/ratelimits/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xbox/webapi/common/ratelimits/__init__.py b/xbox/webapi/common/ratelimits/__init__.py index 50d903e..42276db 100644 --- a/xbox/webapi/common/ratelimits/__init__.py +++ b/xbox/webapi/common/ratelimits/__init__.py @@ -97,7 +97,7 @@ def __set_reset_after(self): ) -class CombinedRateLimit: +class CombinedRateLimit(RateLimit): def __init__(self, *parsed_limits: ParsedRateLimit, type: LimitType): # *parsed_limits is a tuple From c956f816dfc71e7564afbe87c236068cc7e8c3cd Mon Sep 17 00:00:00 2001 From: James Cahill Date: Tue, 15 Nov 2022 12:04:57 +0000 Subject: [PATCH 34/48] (CombinedRateLimit) add get_limits, get_limits_by_period funcs --- xbox/webapi/common/ratelimits/__init__.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/xbox/webapi/common/ratelimits/__init__.py b/xbox/webapi/common/ratelimits/__init__.py index 42276db..9731535 100644 --- a/xbox/webapi/common/ratelimits/__init__.py +++ b/xbox/webapi/common/ratelimits/__init__.py @@ -170,6 +170,15 @@ def get_reset_after(self) -> Union[datetime, None]: # dates_valid has no elements, return None return None + def get_limits(self) -> list[SingleRateLimit]: + return self.__limits + + def get_limits_by_period(self, period: TimePeriod) -> list[SingleRateLimit]: + # Filter the list for the given LimitType + matches = filter(lambda limit: limit.get_time_period() == period, self.__limits) + # Convert the filter object to a list and return it + return list(matches) + def is_exceeded(self) -> bool: # Map self.__limits to (limit).is_exceeded() is_exceeded_map = map(lambda limit: limit.is_exceeded(), self.__limits) From 76c17608cd989b15ff031ae73d0a09e5317a3b72 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Tue, 15 Nov 2022 12:06:59 +0000 Subject: [PATCH 35/48] (SingleRateLimit) add get_limit func --- xbox/webapi/common/ratelimits/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/xbox/webapi/common/ratelimits/__init__.py b/xbox/webapi/common/ratelimits/__init__.py index 9731535..350a62a 100644 --- a/xbox/webapi/common/ratelimits/__init__.py +++ b/xbox/webapi/common/ratelimits/__init__.py @@ -46,6 +46,9 @@ def get_counter(self) -> int: def get_time_period(self) -> "TimePeriod": return self.__time_period + def get_limit(self) -> int: + return self.__limit + def get_limit_type(self) -> "LimitType": return self.__type From 2c2633e33b95df075ea6c44b14e41f86d933abbf Mon Sep 17 00:00:00 2001 From: James Cahill Date: Tue, 15 Nov 2022 12:08:02 +0000 Subject: [PATCH 36/48] Test parsing of RATE_LIMITS in different formats --- tests/test_ratelimits.py | 64 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/tests/test_ratelimits.py b/tests/test_ratelimits.py index cbe4070..38722b9 100644 --- a/tests/test_ratelimits.py +++ b/tests/test_ratelimits.py @@ -3,8 +3,72 @@ import pytest from tests.common import get_response_json +from xbox.webapi.api.provider.ratelimitedprovider import RateLimitedProvider from xbox.webapi.common.exceptions import RateLimitExceededException +from xbox.webapi.common.ratelimits import CombinedRateLimit +from xbox.webapi.common.ratelimits.models import TimePeriod + + +def helper_test_combinedratelimit( + crl: CombinedRateLimit, burstLimit: int, sustainLimit: int +): + burst = crl.get_limits_by_period(TimePeriod.BURST) + sustain = crl.get_limits_by_period(TimePeriod.SUSTAIN) + + # These functions should return a list with one element + assert type(burst) == list + assert type(sustain) == list + + assert len(burst) == 1 + assert len(sustain) == 1 + + # Check that their limits are what we expect + assert burst[0].get_limit() == burstLimit + assert sustain[0].get_limit() == sustainLimit + + +def test_ratelimitedprovider_rate_limits_same_rw_values(xbl_client): + class child_class(RateLimitedProvider): + RATE_LIMITS = {"burst": 1, "sustain": 2} + + instance = child_class(xbl_client) + + helper_test_combinedratelimit(instance.rate_limit_read, 1, 2) + helper_test_combinedratelimit(instance.rate_limit_write, 1, 2) + + +def test_ratelimitedprovider_rate_limits_diff_rw_values(xbl_client): + class child_class(RateLimitedProvider): + RATE_LIMITS = { + "burst": {"read": 1, "write": 2}, + "sustain": {"read": 3, "write": 4}, + } + + instance = child_class(xbl_client) + + helper_test_combinedratelimit(instance.rate_limit_read, 1, 3) + helper_test_combinedratelimit(instance.rate_limit_write, 2, 4) + + +def test_ratelimitedprovider_rate_limits_mixed(xbl_client): + class burst_diff(RateLimitedProvider): + RATE_LIMITS = {"burst": {"read": 1, "write": 2}, "sustain": 3} + + burst_diff_inst = burst_diff(xbl_client) + + # Sustain values are the same (third paramater) + helper_test_combinedratelimit(burst_diff_inst.rate_limit_read, 1, 3) + helper_test_combinedratelimit(burst_diff_inst.rate_limit_write, 2, 3) + + class sustain_diff(RateLimitedProvider): + RATE_LIMITS = {"burst": 4, "sustain": {"read": 5, "write": 6}} + + sustain_diff_inst = sustain_diff(xbl_client) + + # Burst values are the same (second paramater) + helper_test_combinedratelimit(sustain_diff_inst.rate_limit_read, 4, 5) + helper_test_combinedratelimit(sustain_diff_inst.rate_limit_write, 4, 6) @pytest.mark.asyncio From fcbeb7c2f7f342360cadd458893b4a1d0cf1b627 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Tue, 15 Nov 2022 12:11:53 +0000 Subject: [PATCH 37/48] Use typing.List for type hints (fix for py <3.9) --- xbox/webapi/common/ratelimits/__init__.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/xbox/webapi/common/ratelimits/__init__.py b/xbox/webapi/common/ratelimits/__init__.py index 350a62a..42d4ec6 100644 --- a/xbox/webapi/common/ratelimits/__init__.py +++ b/xbox/webapi/common/ratelimits/__init__.py @@ -1,5 +1,5 @@ from datetime import datetime, timedelta -from typing import Union +from typing import Union, List from xbox.webapi.common.ratelimits.models import ( ParsedRateLimit, @@ -173,10 +173,12 @@ def get_reset_after(self) -> Union[datetime, None]: # dates_valid has no elements, return None return None - def get_limits(self) -> list[SingleRateLimit]: + # list -> List (typing.List) https://stackoverflow.com/a/63460173 + def get_limits(self) -> List[SingleRateLimit]: return self.__limits - def get_limits_by_period(self, period: TimePeriod) -> list[SingleRateLimit]: + # list -> List (typing.List) https://stackoverflow.com/a/63460173 + def get_limits_by_period(self, period: TimePeriod) -> List[SingleRateLimit]: # Filter the list for the given LimitType matches = filter(lambda limit: limit.get_time_period() == period, self.__limits) # Convert the filter object to a list and return it From 4b9fca0bf47fddaf9511df0275f60e024846eb3e Mon Sep 17 00:00:00 2001 From: James Cahill Date: Tue, 15 Nov 2022 12:43:43 +0000 Subject: [PATCH 38/48] Add error handling for RateLimitedProvider init --- .../api/provider/ratelimitedprovider.py | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/xbox/webapi/api/provider/ratelimitedprovider.py b/xbox/webapi/api/provider/ratelimitedprovider.py index 58dab1f..2661d68 100644 --- a/xbox/webapi/api/provider/ratelimitedprovider.py +++ b/xbox/webapi/api/provider/ratelimitedprovider.py @@ -6,6 +6,7 @@ from typing import Union, Dict from xbox.webapi.api.provider.baseprovider import BaseProvider +from xbox.webapi.common.exceptions import XboxException from xbox.webapi.common.ratelimits.models import LimitType, ParsedRateLimit, TimePeriod from xbox.webapi.common.ratelimits import CombinedRateLimit @@ -23,6 +24,23 @@ def __init__(self, client): """ super().__init__(client) + # Check that RATE_LIMITS set defined in the child class + if hasattr(self, "RATE_LIMITS"): + # Note: we cannot check (type(self.RATE_LIMITS) == dict) as the type hints have already defined it as such + if "burst" and "sustain" in self.RATE_LIMITS: + # We have the required keys, attempt to parse. + # (type-checking for the values is performed in __parse_rate_limit_key) + self.__handle_rate_limit_setup() + else: + raise XboxException( + "RATE_LIMITS object missing required keys 'burst', 'sustain'" + ) + else: + raise XboxException( + "RateLimitedProvider as parent class but RATE_LIMITS not set!" + ) + + def __handle_rate_limit_setup(self): # Retrieve burst and sustain from the dict burst_key = self.RATE_LIMITS["burst"] sustain_key = self.RATE_LIMITS["sustain"] @@ -52,3 +70,7 @@ def __parse_rate_limit_key( # Since the key-value pairs match we can just pass the dict to the model return ParsedRateLimit(**key, period=period) # return ParsedRateLimit(read=key["read"], write=key["write"]) + else: + raise XboxException( + "RATE_LIMITS value types not recognised. Must be one of 'int, 'dict'." + ) From c2ee539b20c222fec2475fb0485bf5a836ba4674 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Tue, 15 Nov 2022 12:44:16 +0000 Subject: [PATCH 39/48] Add tests for RateLimitProvider error handling --- tests/test_ratelimits.py | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/tests/test_ratelimits.py b/tests/test_ratelimits.py index 38722b9..6508b39 100644 --- a/tests/test_ratelimits.py +++ b/tests/test_ratelimits.py @@ -5,7 +5,7 @@ from tests.common import get_response_json from xbox.webapi.api.provider.ratelimitedprovider import RateLimitedProvider -from xbox.webapi.common.exceptions import RateLimitExceededException +from xbox.webapi.common.exceptions import RateLimitExceededException, XboxException from xbox.webapi.common.ratelimits import CombinedRateLimit from xbox.webapi.common.ratelimits.models import TimePeriod @@ -71,6 +71,39 @@ class sustain_diff(RateLimitedProvider): helper_test_combinedratelimit(sustain_diff_inst.rate_limit_write, 4, 6) +def test_ratelimitedprovider_rate_limits_missing_values_correct_type(xbl_client): + class child_class(RateLimitedProvider): + RATE_LIMITS = {"incorrect": "values"} + + with pytest.raises(XboxException) as exception: + child_class(xbl_client) + + ex: XboxException = exception.value + assert "RATE_LIMITS object missing required keys" in ex.args[0] + + +def test_ratelimitedprovider_rate_limits_not_set(xbl_client): + class child_class(RateLimitedProvider): + pass + + with pytest.raises(XboxException) as exception: + child_class(xbl_client) + + ex: XboxException = exception.value + assert "RateLimitedProvider as parent class but RATE_LIMITS not set!" in ex.args[0] + + +def test_ratelimitedprovider_rate_limits_incorrect_key_type(xbl_client): + class child_class(RateLimitedProvider): + RATE_LIMITS = {"burst": True, "sustain": False} + + with pytest.raises(XboxException) as exception: + child_class(xbl_client) + + ex: XboxException = exception.value + assert "RATE_LIMITS value types not recognised." in ex.args[0] + + @pytest.mark.asyncio async def test_ratelimits_exceeded_burst_only(respx_mock, xbl_client): async def make_request(): From 74adc453d0a5d626a8f95526214e3c98158cb7e8 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Tue, 15 Nov 2022 13:05:12 +0000 Subject: [PATCH 40/48] Rate limits are only known for social.xboxlive.com This commit removes the rate_limits passing from get_friends_own (peoplehub.xboxlive.com) as it is not explicitly known what rate limits apply to this domain. This commit adds rate_limits passing all to functions that call social.xboxlive.com: - get_friends_summary_own - get_friends_summary_by_xuid - get_friends_summary_by_gamertag This commit also updates the test 'test_ratelimits_exceeded_burst_only' to use a function calling social.xboxlive.com (Test would otherwise break as previous function no longer passes rate_limits) --- tests/test_ratelimits.py | 7 +++---- xbox/webapi/api/provider/people/__init__.py | 17 +++++++++++------ 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/tests/test_ratelimits.py b/tests/test_ratelimits.py index 6508b39..17e1a2b 100644 --- a/tests/test_ratelimits.py +++ b/tests/test_ratelimits.py @@ -107,12 +107,11 @@ class child_class(RateLimitedProvider): @pytest.mark.asyncio async def test_ratelimits_exceeded_burst_only(respx_mock, xbl_client): async def make_request(): - route = respx_mock.get("https://peoplehub.xboxlive.com").mock( - return_value=Response(200, json=get_response_json("people_friends_own")) + route = respx_mock.get("https://social.xboxlive.com").mock( + return_value=Response(200, json=get_response_json("people_summary_own")) ) - ret = await xbl_client.people.get_friends_own() + ret = await xbl_client.people.get_friends_summary_own() - assert len(ret.people) == 2 assert route.called # Record the start time to ensure that the timeouts are the correct length diff --git a/xbox/webapi/api/provider/people/__init__.py b/xbox/webapi/api/provider/people/__init__.py index 72fd037..93d3882 100644 --- a/xbox/webapi/api/provider/people/__init__.py +++ b/xbox/webapi/api/provider/people/__init__.py @@ -21,6 +21,7 @@ class PeopleProvider(RateLimitedProvider): } SEPERATOR = "," + # NOTE: Rate Limits are noted for social.xboxlive.com ONLY RATE_LIMITS = {"burst": 10, "sustain": 30} def __init__(self, client): @@ -53,9 +54,7 @@ async def get_friends_own( decoration = self.SEPERATOR.join(decoration_fields) url = f"{self.PEOPLE_URL}/users/me/people/social/decoration/{decoration}" - resp = await self.client.session.get( - url, headers=self._headers, rate_limits=self.rate_limit_read, **kwargs - ) + resp = await self.client.session.get(url, headers=self._headers, **kwargs) resp.raise_for_status() return PeopleResponse(**resp.json()) @@ -133,7 +132,9 @@ async def get_friends_summary_own(self, **kwargs) -> PeopleSummaryResponse: :class:`PeopleSummaryResponse`: People Summary Response """ url = self.SOCIAL_URL + "/users/me/summary" - resp = await self.client.session.get(url, headers=self.HEADERS_SOCIAL, **kwargs) + resp = await self.client.session.get( + url, headers=self.HEADERS_SOCIAL, rate_limits=self.rate_limit_read, **kwargs + ) resp.raise_for_status() return PeopleSummaryResponse(**resp.json()) @@ -150,7 +151,9 @@ async def get_friends_summary_by_xuid( :class:`PeopleSummaryResponse`: People Summary Response """ url = self.SOCIAL_URL + f"/users/xuid({xuid})/summary" - resp = await self.client.session.get(url, headers=self.HEADERS_SOCIAL, **kwargs) + resp = await self.client.session.get( + url, headers=self.HEADERS_SOCIAL, rate_limits=self.rate_limit_read, **kwargs + ) resp.raise_for_status() return PeopleSummaryResponse(**resp.json()) @@ -167,6 +170,8 @@ async def get_friends_summary_by_gamertag( :class:`PeopleSummaryResponse`: People Summary Response """ url = self.SOCIAL_URL + f"/users/gt({gamertag})/summary" - resp = await self.client.session.get(url, headers=self.HEADERS_SOCIAL, **kwargs) + resp = await self.client.session.get( + url, headers=self.HEADERS_SOCIAL, rate_limits=self.rate_limit_read, **kwargs + ) resp.raise_for_status() return PeopleSummaryResponse(**resp.json()) From 0f9552ddab0684a9f8a3e86d07c32f711eeefcfb Mon Sep 17 00:00:00 2001 From: James Cahill Date: Tue, 15 Nov 2022 13:22:38 +0000 Subject: [PATCH 41/48] (AchievementsProvider): Add rate limits --- .../api/provider/achievements/__init__.py | 39 +++++++++++++++---- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/xbox/webapi/api/provider/achievements/__init__.py b/xbox/webapi/api/provider/achievements/__init__.py index 106bcba..27111ec 100644 --- a/xbox/webapi/api/provider/achievements/__init__.py +++ b/xbox/webapi/api/provider/achievements/__init__.py @@ -9,14 +9,16 @@ AchievementResponse, RecentProgressResponse, ) -from xbox.webapi.api.provider.baseprovider import BaseProvider +from xbox.webapi.api.provider.ratelimitedprovider import RateLimitedProvider -class AchievementsProvider(BaseProvider): +class AchievementsProvider(RateLimitedProvider): ACHIEVEMENTS_URL = "https://achievements.xboxlive.com" HEADERS_GAME_360_PROGRESS = {"x-xbl-contract-version": "1"} HEADERS_GAME_PROGRESS = {"x-xbl-contract-version": "2"} + RATE_LIMITS = {"burst": 100, "sustain": 300} + async def get_achievements_detail_item( self, xuid, service_config_id, achievement_id, **kwargs ) -> AchievementResponse: @@ -33,7 +35,10 @@ async def get_achievements_detail_item( """ url = f"{self.ACHIEVEMENTS_URL}/users/xuid({xuid})/achievements/{service_config_id}/{achievement_id}" resp = await self.client.session.get( - url, headers=self.HEADERS_GAME_PROGRESS, **kwargs + url, + headers=self.HEADERS_GAME_PROGRESS, + rate_limits=self.rate_limit_read, + **kwargs, ) resp.raise_for_status() return AchievementResponse(**resp.json()) @@ -54,7 +59,11 @@ async def get_achievements_xbox360_all( url = f"{self.ACHIEVEMENTS_URL}/users/xuid({xuid})/titleachievements?" params = {"titleId": title_id} resp = await self.client.session.get( - url, params=params, headers=self.HEADERS_GAME_360_PROGRESS, **kwargs + url, + params=params, + headers=self.HEADERS_GAME_360_PROGRESS, + rate_limits=self.rate_limit_read, + **kwargs, ) resp.raise_for_status() return Achievement360Response(**resp.json()) @@ -75,7 +84,11 @@ async def get_achievements_xbox360_earned( url = f"{self.ACHIEVEMENTS_URL}/users/xuid({xuid})/achievements?" params = {"titleId": title_id} resp = await self.client.session.get( - url, params=params, headers=self.HEADERS_GAME_360_PROGRESS, **kwargs + url, + params=params, + headers=self.HEADERS_GAME_360_PROGRESS, + rate_limits=self.rate_limit_read, + **kwargs, ) resp.raise_for_status() return Achievement360Response(**resp.json()) @@ -94,7 +107,10 @@ async def get_achievements_xbox360_recent_progress_and_info( """ url = f"{self.ACHIEVEMENTS_URL}/users/xuid({xuid})/history/titles" resp = await self.client.session.get( - url, headers=self.HEADERS_GAME_360_PROGRESS, **kwargs + url, + headers=self.HEADERS_GAME_360_PROGRESS, + rate_limits=self.rate_limit_read, + **kwargs, ) resp.raise_for_status() return Achievement360ProgressResponse(**resp.json()) @@ -115,7 +131,11 @@ async def get_achievements_xboxone_gameprogress( url = f"{self.ACHIEVEMENTS_URL}/users/xuid({xuid})/achievements?" params = {"titleId": title_id} resp = await self.client.session.get( - url, params=params, headers=self.HEADERS_GAME_PROGRESS, **kwargs + url, + params=params, + headers=self.HEADERS_GAME_PROGRESS, + rate_limits=self.rate_limit_read, + **kwargs, ) resp.raise_for_status() return AchievementResponse(**resp.json()) @@ -134,7 +154,10 @@ async def get_achievements_xboxone_recent_progress_and_info( """ url = f"{self.ACHIEVEMENTS_URL}/users/xuid({xuid})/history/titles" resp = await self.client.session.get( - url, headers=self.HEADERS_GAME_PROGRESS, **kwargs + url, + headers=self.HEADERS_GAME_PROGRESS, + rate_limits=self.rate_limit_read, + **kwargs, ) resp.raise_for_status() return RecentProgressResponse(**resp.json()) From 683055e535bc0a98684bebb0e3cdbd0aae494a7a Mon Sep 17 00:00:00 2001 From: James Cahill Date: Tue, 15 Nov 2022 13:23:30 +0000 Subject: [PATCH 42/48] (ProfileProvider): Add rate limits --- xbox/webapi/api/provider/profile/__init__.py | 24 ++++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/xbox/webapi/api/provider/profile/__init__.py b/xbox/webapi/api/provider/profile/__init__.py index cad9dd0..4cce30b 100644 --- a/xbox/webapi/api/provider/profile/__init__.py +++ b/xbox/webapi/api/provider/profile/__init__.py @@ -5,15 +5,17 @@ """ from typing import List -from xbox.webapi.api.provider.baseprovider import BaseProvider +from xbox.webapi.api.provider.ratelimitedprovider import RateLimitedProvider from xbox.webapi.api.provider.profile.models import ProfileResponse, ProfileSettings -class ProfileProvider(BaseProvider): +class ProfileProvider(RateLimitedProvider): PROFILE_URL = "https://profile.xboxlive.com" HEADERS_PROFILE = {"x-xbl-contract-version": "3"} SEPARATOR = "," + RATE_LIMITS = {"burst": 10, "sustain": 30} + async def get_profiles(self, xuid_list: List[str], **kwargs) -> ProfileResponse: """ Get profile info for list of xuids @@ -45,7 +47,11 @@ async def get_profiles(self, xuid_list: List[str], **kwargs) -> ProfileResponse: } url = self.PROFILE_URL + "/users/batch/profile/settings" resp = await self.client.session.post( - url, json=post_data, headers=self.HEADERS_PROFILE, **kwargs + url, + json=post_data, + headers=self.HEADERS_PROFILE, + rate_limits=self.rate_limit_read, + **kwargs, ) resp.raise_for_status() return ProfileResponse(**resp.json()) @@ -83,7 +89,11 @@ async def get_profile_by_xuid(self, target_xuid: str, **kwargs) -> ProfileRespon ) } resp = await self.client.session.get( - url, params=params, headers=self.HEADERS_PROFILE, **kwargs + url, + params=params, + headers=self.HEADERS_PROFILE, + rate_limits=self.rate_limit_read, + **kwargs, ) resp.raise_for_status() return ProfileResponse(**resp.json()) @@ -121,7 +131,11 @@ async def get_profile_by_gamertag(self, gamertag: str, **kwargs) -> ProfileRespo ) } resp = await self.client.session.get( - url, params=params, headers=self.HEADERS_PROFILE, **kwargs + url, + params=params, + headers=self.HEADERS_PROFILE, + rate_limits=self.rate_limit_read, + **kwargs, ) resp.raise_for_status() return ProfileResponse(**resp.json()) From 01c8dbe7af0c4cf787346f6e767c35d0631d2525 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Tue, 15 Nov 2022 13:24:45 +0000 Subject: [PATCH 43/48] (UserStatsProvider): Add rate limits --- .../webapi/api/provider/userstats/__init__.py | 32 +++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/xbox/webapi/api/provider/userstats/__init__.py b/xbox/webapi/api/provider/userstats/__init__.py index ed1f52c..4ad4151 100644 --- a/xbox/webapi/api/provider/userstats/__init__.py +++ b/xbox/webapi/api/provider/userstats/__init__.py @@ -3,19 +3,24 @@ """ from typing import List, Optional -from xbox.webapi.api.provider.baseprovider import BaseProvider +from xbox.webapi.api.provider.ratelimitedprovider import RateLimitedProvider from xbox.webapi.api.provider.userstats.models import ( GeneralStatsField, UserStatsResponse, ) -class UserStatsProvider(BaseProvider): +class UserStatsProvider(RateLimitedProvider): USERSTATS_URL = "https://userstats.xboxlive.com" HEADERS_USERSTATS = {"x-xbl-contract-version": "2"} HEADERS_USERSTATS_WITH_METADATA = {"x-xbl-contract-version": "3"} SEPERATOR = "," + # NOTE: Stats Read (userstats.xboxlive.com) and Stats Write (statswrite.xboxlive.com) + # Are mentioned as their own objects but their rate limits are the same and do not collide + # (Stats Read -> read rate limit, Stats Write -> write rate limit) + RATE_LIMITS = {"burst": 100, "sustain": 300} + async def get_stats( self, xuid: str, @@ -40,7 +45,10 @@ async def get_stats( url = f"{self.USERSTATS_URL}/users/xuid({xuid})/scids/{service_config_id}/stats/{stats}" resp = await self.client.session.get( - url, headers=self.HEADERS_USERSTATS, **kwargs + url, + headers=self.HEADERS_USERSTATS, + rate_limits=self.rate_limit_read, + **kwargs, ) resp.raise_for_status() return UserStatsResponse(**resp.json()) @@ -70,7 +78,11 @@ async def get_stats_with_metadata( url = f"{self.USERSTATS_URL}/users/xuid({xuid})/scids/{service_config_id}/stats/{stats}" params = {"include": "valuemetadata"} resp = await self.client.session.get( - url, params=params, headers=self.HEADERS_USERSTATS_WITH_METADATA, **kwargs + url, + params=params, + headers=self.HEADERS_USERSTATS_WITH_METADATA, + rate_limits=self.rate_limit_read, + **kwargs, ) resp.raise_for_status() return UserStatsResponse(**resp.json()) @@ -104,7 +116,11 @@ async def get_stats_batch( "xuids": xuids, } resp = await self.client.session.post( - url, json=post_data, headers=self.HEADERS_USERSTATS, **kwargs + url, + json=post_data, + headers=self.HEADERS_USERSTATS, + rate_limits=self.rate_limit_read, + **kwargs, ) resp.raise_for_status() return UserStatsResponse(**resp.json()) @@ -139,7 +155,11 @@ async def get_stats_batch_by_scid( "xuids": xuids, } resp = await self.client.session.post( - url, json=post_data, headers=self.HEADERS_USERSTATS, **kwargs + url, + json=post_data, + headers=self.HEADERS_USERSTATS, + rate_limits=self.rate_limit_read, + **kwargs, ) resp.raise_for_status() return UserStatsResponse(**resp.json()) From 8c4095d6699269b1a173ecc5f68886823753d552 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Tue, 29 Nov 2022 13:19:54 +0000 Subject: [PATCH 44/48] Add sustain only rate limit exceed test, use timeperiod values instead of hardcoded ints --- tests/test_ratelimits.py | 103 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 102 insertions(+), 1 deletion(-) diff --git a/tests/test_ratelimits.py b/tests/test_ratelimits.py index 17e1a2b..264615d 100644 --- a/tests/test_ratelimits.py +++ b/tests/test_ratelimits.py @@ -1,6 +1,7 @@ from datetime import datetime, timedelta from httpx import Response import pytest +import asyncio from tests.common import get_response_json from xbox.webapi.api.provider.ratelimitedprovider import RateLimitedProvider @@ -137,4 +138,104 @@ async def make_request(): # Assert that the timeout is the correct length delta: timedelta = try_again_in - start_time - assert delta.seconds == 15 + assert delta.seconds == TimePeriod.BURST.value # 15 seconds + + +async def helper_reach_and_wait_for_burst( + make_request, start_time, burst_limit: int, expected_counter: int +): + # Make as many requests as possible without exceeding the BURST limit. + for i in range(burst_limit): + await make_request() + + # Make another request, ensure that it raises the exception. + with pytest.raises(RateLimitExceededException) as exception: + await make_request() + + # Get the error instance from pytest + ex: RateLimitExceededException = exception.value + + # Assert that the counter matches the what we expect (burst, 2x burstm etc) + assert ex.rate_limit.get_counter() == expected_counter + + # Get the reset_after value + # (if we call it after waiting for it to expire the function will return None) + burst_resets_after = ex.rate_limit.get_reset_after() + + # Wait for the burst limit timeout to elapse. + await asyncio.sleep(TimePeriod.BURST.value) # 15 seconds + + # Assert that the reset_after value has passed. + assert burst_resets_after < datetime.now() + + +@pytest.mark.asyncio +async def test_ratelimits_exceeded_sustain_only(respx_mock, xbl_client): + async def make_request(): + route = respx_mock.get("https://social.xboxlive.com").mock( + return_value=Response(200, json=get_response_json("people_summary_own")) + ) + ret = await xbl_client.people.get_friends_summary_own() + + assert route.called + + # Record the start time to ensure that the timeouts are the correct length + start_time = datetime.now() + + # Get the max requests for this route. + max_request_num = xbl_client.people.RATE_LIMITS["sustain"] # 30 + burst_max_request_num = xbl_client.people.RATE_LIMITS["burst"] # 10 + + # In this case, the BURST limit is three times that of SUSTAIN, so we need to exceed the burst limit three times. + + # Exceed the burst limit and wait for it to reset (10 requests) + await helper_reach_and_wait_for_burst( + make_request, start_time, burst_limit=burst_max_request_num, expected_counter=10 + ) + + # Repeat: Exceed the burst limit and wait for it to reset (10 requests) + # Counter (the sustain one will be returned) + # For (CombinedRateLimit).get_counter(), the highest counter is returned. (sustain in this case) + await helper_reach_and_wait_for_burst( + make_request, start_time, burst_limit=burst_max_request_num, expected_counter=20 + ) + + # Now, make the rest of the requests (10 left, 20/30 done!) + for i in range(10): + await make_request() + + # Wait for the burst limit to 'reset'. + await asyncio.sleep(TimePeriod.BURST.value) # 15 seconds + + # Now, we have made 30 requests. + # The counters should be as follows: + # - BURST: 0* (will reset on next check) + # - SUSTAIN: 30 + # The next request we make should exceed the SUSTAIN rate limit. + + # Make another request, ensure that it raises the exception. + with pytest.raises(RateLimitExceededException) as exception: + await make_request() + + # Get the error instance from pytest + ex: RateLimitExceededException = exception.value + + # Get the SingleRateLimit objects from the exception + rl: CombinedRateLimit = ex.rate_limit + burst = rl.get_limits_by_period(TimePeriod.BURST)[0] + sustain = rl.get_limits_by_period(TimePeriod.SUSTAIN)[0] + + # Assert that we have only exceeded the sustain limit. + assert not burst.is_exceeded() + assert sustain.is_exceeded() + + # Assert that the counter matches the max request num (should not have incremented above max value) + assert ex.rate_limit.get_counter() == max_request_num + + # Get the timeout we were issued + try_again_in = ex.rate_limit.get_reset_after() + + # Assert that the timeout is the correct length + # The SUSTAIN counter has not been reset during this test, so the try again in should be 300 seconds since we started this test. + delta: timedelta = try_again_in - start_time + assert delta.seconds == TimePeriod.SUSTAIN.value # 300 seconds (5 minutes) From 372960090ea3439beb131bb420b3eafbdbb24258 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Mon, 16 Jan 2023 12:07:56 +0000 Subject: [PATCH 45/48] Add initial docstrings to RateLimit classes --- xbox/webapi/common/ratelimits/__init__.py | 72 +++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/xbox/webapi/common/ratelimits/__init__.py b/xbox/webapi/common/ratelimits/__init__.py index 42d4ec6..b8e837d 100644 --- a/xbox/webapi/common/ratelimits/__init__.py +++ b/xbox/webapi/common/ratelimits/__init__.py @@ -12,24 +12,55 @@ class RateLimit(metaclass=ABCMeta): + """ + Abstract class for varying implementations/types of rate limits. + All methods in this class are overriden in every implementation. + However, different implementations may have additional functions not present in this parent abstract class. + + A class implementing RateLimit functions without any external threads. + When the first increment request is recieved (after a counter reset or a new instaniciation) + a reset_after variable is set detailing when the rate limit(s) reset. + + Upon each function invokation, the reset_after variable is checked and the timer is automatically reset if the reset_after time has passed. + """ + @abstractmethod def get_counter(self) -> int: + # Docstrings are defined in child classes due to their differing implementations. pass @abstractmethod def get_reset_after(self) -> Union[datetime, None]: + # Docstrings are defined in child classes due to their differing implementations. pass @abstractmethod def is_exceeded(self) -> bool: + # Docstrings are defined in child classes due to their differing implementations. pass @abstractmethod def increment(self) -> IncrementResult: + """ + The increment function adds one to the rate limit request counter. + + If the reset_after time has passed, the counter will first be reset before counting the request. + + When the counter hits 1, the reset_after time is calculated and stored. + + This function returns an `IncrementResult` object, containing the keys `counter: int` and `exceeded: bool`. + This can be used by the caller to determine the current state of the rate-limit object without making an additional function call. + """ + pass class SingleRateLimit(RateLimit): + """ + A rate limit implementation for a single rate limit, such as a burst or sustain limit. + This class is mainly used by the CombinedRateLimit class. + """ + def __init__(self, time_period: TimePeriod, type: LimitType, limit: int): self.__time_period = time_period self.__type = type @@ -41,6 +72,10 @@ def __init__(self, time_period: TimePeriod, type: LimitType, limit: int): self.__reset_after: Union[datetime, None] = None def get_counter(self) -> int: + """ + This function returns the current request counter variable. + """ + return self.__counter def get_time_period(self) -> "TimePeriod": @@ -53,9 +88,21 @@ def get_limit_type(self) -> "LimitType": return self.__type def get_reset_after(self) -> Union[datetime, None]: + """ + This getter returns the current state of the reset_after counter. + + If the counter in use, it's corresponding `datetime` object is returned. + + If the counter is not in use, `None` is returned. + """ + return self.__reset_after def is_exceeded(self) -> bool: + """ + This functions returns `True` if the rate limit has been exceeded. + """ + self.__reset_counter_if_required() return self.__exceeded @@ -101,6 +148,11 @@ def __set_reset_after(self): class CombinedRateLimit(RateLimit): + """ + A rate limit implementation for multiple rate limits, such as burst and sustain. + + """ + def __init__(self, *parsed_limits: ParsedRateLimit, type: LimitType): # *parsed_limits is a tuple @@ -123,6 +175,12 @@ def __init__(self, *parsed_limits: ParsedRateLimit, type: LimitType): ) def get_counter(self) -> int: + """ + This function returns the request counter with the **highest** value. + + A `CombinedRateLimit` consists of multiple different rate limits, which may have differing counter values. + """ + # Map self.__limits to (limit).get_counter() counter_map = map(lambda limit: limit.get_counter(), self.__limits) counters = list(counter_map) @@ -137,6 +195,16 @@ def get_counter(self) -> int: # We don't want a datetime response for a limit that has not been exceeded. # Otherwise eg. 10 burst requests -> 300s timeout (should be 30 (burst exceeded), 300s (not exceeded) def get_reset_after(self) -> Union[datetime, None]: + """ + This getter returns either a `datetime` object or `None` object depending on the status of the rate limit. + + If the counter is in use, the rate limit with the **latest** reset_after is returned. + + This is so that this function can reliably be used as a indicator of when all rate limits have been reset. + + If the counter is not in use, `None` is returned. + """ + # Get a list of limits that *have been exceeded* dates_exceeded_only = filter(lambda limit: limit.is_exceeded(), self.__limits) @@ -185,6 +253,10 @@ def get_limits_by_period(self, period: TimePeriod) -> List[SingleRateLimit]: return list(matches) def is_exceeded(self) -> bool: + """ + This function returns `True` if **any** rate limit has been exceeded. + """ + # Map self.__limits to (limit).is_exceeded() is_exceeded_map = map(lambda limit: limit.is_exceeded(), self.__limits) is_exceeded_list = list(is_exceeded_map) From eed421b55c8ae29dee1a24bf535e6e8facd34771 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Thu, 9 Mar 2023 12:14:12 +0000 Subject: [PATCH 46/48] Don't overwrite results[1].counter Likely a change made while developing that I forgot to remove. --- xbox/webapi/common/ratelimits/__init__.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/xbox/webapi/common/ratelimits/__init__.py b/xbox/webapi/common/ratelimits/__init__.py index b8e837d..962abf5 100644 --- a/xbox/webapi/common/ratelimits/__init__.py +++ b/xbox/webapi/common/ratelimits/__init__.py @@ -271,11 +271,7 @@ def increment(self) -> IncrementResult: result = limit.increment() results.append(result) - # SPEC: Which counter should be picked here? - # For now, let's pick the *higher* counter - # (should incrementResult even include the counter?) - results[1].counter = 5 - + # SPEC: Let's pick the *higher* counter # By default, sorted() returns in ascending order, so let's set reverse=True # This means that the result with the highest counter will be the first element. results_sorted = sorted(results, key=lambda i: i.counter, reverse=True) From 131c9131bf94580f607f897547c1ea1a48bc1bf4 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Thu, 9 Mar 2023 12:18:51 +0000 Subject: [PATCH 47/48] Remove debug print statements from __init__.py --- xbox/webapi/common/ratelimits/__init__.py | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/xbox/webapi/common/ratelimits/__init__.py b/xbox/webapi/common/ratelimits/__init__.py index 962abf5..1fd2d3f 100644 --- a/xbox/webapi/common/ratelimits/__init__.py +++ b/xbox/webapi/common/ratelimits/__init__.py @@ -167,13 +167,6 @@ def __init__(self, *parsed_limits: ParsedRateLimit, type: LimitType): srl = SingleRateLimit(limit.period, type, limit_num) self.__limits.append(srl) - # DEBUG: print them - for i in self.__limits: - print( - "Added limit of type %s, limit %s, and limit %i" - % (i.get_limit_type(), i.get_time_period(), i._SingleRateLimit__limit) - ) - def get_counter(self) -> int: """ This function returns the request counter with the **highest** value. @@ -221,20 +214,10 @@ def get_reset_after(self) -> Union[datetime, None]: # If dates_valid has any elements, return the one with the *later* timestamp. # This means that if two or more limits have been exceeded, we wait for both to have reset (by returning the later timestamp) if len(dates_valid) != 0: - dates_valid[0].isoformat - print( - "Valid dates BEFORE sorting: %s" - % list(map(lambda i: i.isoformat(), dates_valid)) - ) # By default dates are sorted with the earliest date first. # We will set reverse=True so that the first element is the later date. dates_valid.sort(reverse=True) - print( - "Valid dates AFTER sorting: %s" - % list(map(lambda i: i.isoformat(), dates_valid)) - ) - # Return the datetime object. return dates_valid[0] From 4f1a6b045cb8b1885dfe2bec98cf21fe4b6100a8 Mon Sep 17 00:00:00 2001 From: James Cahill Date: Thu, 9 Mar 2023 12:24:38 +0000 Subject: [PATCH 48/48] Adjust comment wording for is_exceeded --- xbox/webapi/common/ratelimits/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/xbox/webapi/common/ratelimits/__init__.py b/xbox/webapi/common/ratelimits/__init__.py index 1fd2d3f..9e4e9ec 100644 --- a/xbox/webapi/common/ratelimits/__init__.py +++ b/xbox/webapi/common/ratelimits/__init__.py @@ -238,6 +238,8 @@ def get_limits_by_period(self, period: TimePeriod) -> List[SingleRateLimit]: def is_exceeded(self) -> bool: """ This function returns `True` if **any** rate limit has been exceeded. + + It behaves like an OR logic gate. """ # Map self.__limits to (limit).is_exceeded() @@ -264,5 +266,5 @@ def increment(self) -> IncrementResult: counter=results_sorted[ 0 ].counter, # Use the highest counter (sorted in descending order) - exceeded=self.is_exceeded(), # Call self.is_exceeded (True if any limit has been exceeded, a-la an OR gate.) + exceeded=self.is_exceeded(), # Call self.is_exceeded (True if any limit has been exceeded, like an OR gate.) )