Conversation
We added the package for more convenient development. It allows an IDE make auto complete.
We will check this field and will discard requests if it is set up blocked.
We added the key for more convenient handling of RateLimitInfo in service storages.
We added that ability, because sometimes we want to use another period of time for blocking a specific call, but we want to save a period of time when somebody can do requests of URLs without restrictions.
We need this event for making different things when a block happens.
We should dispatch that event, because some developers want to do additional things when block happens. For example, to register block event into system log.
We added dispatching the event before sending a response. It needs when we want to send another more reach message in the response. Also it allows us to make other types of responses.
This feature allows to block the URL or call for custom period of time which can be not equal period or might be omit at all.
|
This looks like a great feature, if the conflicts could be resolved I'd like to merge it. |
| */ | ||
| public function setKey($key) | ||
| { | ||
| $this->key = (string)$key; |
There was a problem hiding this comment.
I'm not sure why you want to set the key?
There was a problem hiding this comment.
We need the key there.
# Service/Storage/StorageInterface.php
/**
* Set block for the call
*
* @param RateLimitInfo $rateLimitInfo
* @param integer $periodBlock
*
* @return bool
*/
public function setBlock(RateLimitInfo $rateLimitInfo, $periodBlock);If we passed only the key and periodBlock into setBlock method we would have to load cache data before setting block in some storages. It's not a good idea to load data if we have already them.
There was a problem hiding this comment.
I think it would be better for this to work like the other functions in RateLimitService and have $key as the first parameter
# Conflicts: # Service/Storage/DoctrineCache.php # Service/Storage/Memcache.php # composer.json # composer.lock
|
@mcfedr we fixed all the conflicts. |
| if (!$rateLimitInfo->isBlocked() && $rateLimitInfo->getCalls() > $rateLimitInfo->getLimit()) { | ||
| $this->rateLimitService->setBlock( | ||
| $rateLimitInfo, | ||
| $rateLimit->getBlockPeriod() > 0 ? $rateLimit->getBlockPeriod() : $rateLimit->getPeriod() |
There was a problem hiding this comment.
I think If the block period isn't set, there should be no extended block period, as in how system works at the moment.
| */ | ||
| public function setKey($key) | ||
| { | ||
| $this->key = (string)$key; |
There was a problem hiding this comment.
I think it would be better for this to work like the other functions in RateLimitService and have $key as the first parameter
| $info['limit'] = $limit; | ||
| $info['calls'] = 1; | ||
| $info['reset'] = time() + $period; | ||
| $info['blocked'] = 0; |
There was a problem hiding this comment.
blocked is a bool, no need to mix it with ints
3 participants
This changes allow developers to set custom period of blocking time of a call through either an annotation or YAML.