Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Rate limiting updates #141

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Rate limiting updates #141

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
10142fd
using ->end() instead of exit in controllers
abaevbog Jun 5, 2023
3efb9c3
on 429 error, make client wait till full capacity is reached before n…
abaevbog Jun 5, 2023
1758fb9
removed unecessary header setting
abaevbog Jun 8, 2023
3cbf246
default format for mapping action v1/v2 as json
abaevbog Jun 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions controllers/GroupsController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,7 @@ public function groups() {
$group->erase();
Zotero_DB::commit();

header("HTTP/1.1 204 No Content");
exit;
$this->e204();
}


Expand Down Expand Up @@ -486,8 +485,7 @@ public function groupUsers() {

Zotero_DB::commit();

header("HTTP/1.1 204 No Content");
exit;
$this->e204();
}

// Single user
Expand Down
12 changes: 4 additions & 8 deletions controllers/ItemsController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -787,7 +787,6 @@ private function _handleFileRequest($item) {
}
StatsD::increment("storage.view", 1);
$this->redirect($url);
exit;
}

// File download
Expand All @@ -804,7 +803,6 @@ private function _handleFileRequest($item) {

StatsD::increment("storage.download", 1);
$this->redirect($url);
exit;
}

else if ($this->method == 'POST' || $this->method == 'PATCH') {
Expand Down Expand Up @@ -1066,7 +1064,7 @@ private function _handleFileRequest($item) {
header('Content-Type: application/json');
echo json_encode($params);
}
exit;
$this->end();
}

//
Expand Down Expand Up @@ -1128,9 +1126,8 @@ private function _handleFileRequest($item) {

Zotero_DB::commit();

header("HTTP/1.1 204 No Content");
header("Last-Modified-Version: " . $item->version);
exit;
$this->e204();
}


Expand Down Expand Up @@ -1181,12 +1178,11 @@ private function _handleFileRequest($item) {

Zotero_DB::commit();

header("HTTP/1.1 204 No Content");
exit;
$this->e204();
}

throw new Exception("Invalid request", Z_ERROR_INVALID_INPUT);
}
exit;
$this->end();
}
}
18 changes: 4 additions & 14 deletions controllers/KeysController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,7 @@ public function keys() {
$keyObj->erase();
Zotero_DB::commit();

header("HTTP/1.1 204 No Content");
exit;
$this->e204();
}

else {
Expand Down Expand Up @@ -316,20 +315,11 @@ public function keys() {
}
}

if ($this->apiVersion >= 3) {
$this->end();
}
else {
if ($this->apiVersion < 3) {
unset($this->queryParams['format']);
header('Content-Type: application/xml');
$xmlstr = $this->responseXML->asXML();

$doc = new DOMDocument('1.0');

$doc->loadXML($xmlstr);
$doc->formatOutput = true;
echo $doc->saveXML();
exit;
}
$this->end();
}


Expand Down
23 changes: 10 additions & 13 deletions controllers/MappingsController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public function mappings() {
$locale = !empty($_GET['locale'])
? \Zotero\Schema::resolveLocale($_GET['locale'])
: 'en-US';

if ($this->subset == 'itemTypeFields') {
if (empty($_GET['itemType'])) {
$this->e400("'itemType' not provided");
Expand Down Expand Up @@ -63,7 +63,7 @@ public function mappings() {
// Notes and attachments don't have creators
if ($itemType == 'note' || $itemType == 'attachment') {
echo "[]";
exit;
$this->end();
}
}

Expand All @@ -77,9 +77,8 @@ public function mappings() {
$ttl = 60;
$json = Z_Core::$MC->get($cacheKey);
if ($json) {
header("Content-Type: application/json");
echo $json;
exit;
$this->end();
}

switch ($this->subset) {
Expand Down Expand Up @@ -132,20 +131,21 @@ public function mappings() {
);
}

header("Content-Type: application/json");
$json = Zotero_Utilities::formatJSON($json);
Z_Core::$MC->set($cacheKey, $json, $ttl);

echo $json;
exit;
$this->end();
}


public function newItem() {
if (empty($_GET['itemType'])) {
$this->e400("'itemType' not provided");
}


unset($this->queryParams['format']);
header("Content-Type: application/json");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just set 'json' as the default format for the mappings actions for v1/v2 in API.inc.php?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. That is definitely cleaner


$itemType = $_GET['itemType'];
if ($itemType == 'attachment') {
if (empty($_GET['linkMode'])) {
Expand Down Expand Up @@ -200,9 +200,8 @@ public function newItem() {
$ttl = 60;
$json = Z_Core::$MC->get($cacheKey);
if ($json) {
header("Content-Type: application/json");
echo $json;
exit;
$this->end();
}

// Generate template
Expand Down Expand Up @@ -325,12 +324,10 @@ public function newItem() {
unset($json['relations']);
}

header("Content-Type: application/json");

$json = Zotero_Utilities::formatJSON($json);
Z_Core::$MC->set($cacheKey, $json, $ttl);

echo $json;
exit;
$this->end();
}
}
12 changes: 6 additions & 6 deletions controllers/StorageController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public function laststoragesync() {
}

echo $lastSync;
exit;
$this->end();
}


Expand All @@ -66,8 +66,7 @@ public function removestoragefiles() {

$sql = "DELETE FROM storageFileLibraries WHERE libraryID = ?";
Zotero_DB::query($sql, $this->objectLibraryID);
header("HTTP/1.1 204 No Content");
exit;
$this->e204();
}


Expand Down Expand Up @@ -146,9 +145,10 @@ public function storageadmin() {

Zotero_DB::commit();

header('application/xml');
echo $xml->asXML();
exit;
$this->responseXML = $xml;
unset($this->queryParams['format']);
header('Content-Type: application/xml');
$this->end();
}


Expand Down
21 changes: 20 additions & 1 deletion include/RequestLimiter.inc.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,12 @@ public static function checkBucketRate($params) {
return null;
}
$prefix = self::REDIS_PREFIX .'rrl:' . $params['bucket'];

// If bucket should wait for full capacity, reject
if (self::$redis->get($prefix . ".fc")) {
return false;
}

$keys = [$prefix . '.tk', $prefix . '.ts'];
$args = [$params['replenishRate'], $params['capacity'], time()];
try {
Expand All @@ -113,7 +119,14 @@ public static function checkBucketRate($params) {
Z_Core::logError("Warning: " . $e);
return null;
}
return !!$res[0];
// If capacity was reached, save how soon it will be re-filled.
// Requests before that will be rejected.
$allowed = !!$res[0];
if (!$allowed) {
self::addFullCapacityTimeout($prefix, $params['replenishRate'], $params['capacity']);
return false;
}
return true;
}

/**
Expand Down Expand Up @@ -186,4 +199,10 @@ private static function evalLua($lua, $keys, $args) {
}
return $res;
}

//Save how soon a given bucket will be at full capacity
private static function addFullCapacityTimeout($prefix, $rate, $capacity) {
$full_capacity_ts = $capacity / $rate;
self::$redis->setex($prefix . ".fc", (int) $full_capacity_ts, 1);
}
}