Request rate limiter I'm looking for feedback on ways to improve the maintainability and readability of my code.  I'm especially interested in hearing about ways I could improve the code's adherence to good object oriented practices.  Thank you!

Purpose of the code:

Reject a client's requests if that client has already made 100 requests in the last second.  The client who sent a request is identified on the request with a clientID.

Usage:

let rateLimiter = new RateLimiter();

let request = { clientID: 1, time: 0 };

let isRequestAllowed = rateLimiter
  .addRequestAndCheckIsAllowed(request);


Implementation:

const MAX_REQUESTS_IN_TIME_FRAME = 100;
const REQUEST_MAX_AGE = 1000;

class RateLimiter {
  constructor () {
    this._clientRequests = {};
  }

  addRequestAndCheckIsAllowed (request) {
    this._storeRequest(request);

    return this._clientRequestCount(request.clientID) &lt;=
      MAX_REQUESTS_IN_TIME_FRAME;
  }

  _storeRequest (request) {
    const clientID = request.clientID;

    this._clientRequests[clientID] =
      this._clientRequests[clientID] ||
      new RequestBuffer(REQUEST_MAX_AGE);

    this._clientRequests[clientID].store(request);
  }

  _clientRequestCount(clientID) {
    if (this._clientRequests[clientID] === undefined) {
      return 0;
    }

    return this._clientRequests[clientID].count();
  }
};

class RequestBuffer {
  constructor (requestMaxAge) {
    this._requestMaxAge = requestMaxAge;
    this._requests = [];
  }

  store (request) {
    this._requests.push(request);

    const maxTime = Date.now() - this._requestMaxAge;

    this._requests =
      this._requests.filter(request =&gt; request.time &gt; maxTime);
  }

  count () {
    return this._requests.length;
  }
};
Object Oriented Javascript

Many people think that to write OO javascript you need to use the class syntax. This is not at all true. You can write very good OO javascript without ever using the class syntax, including inheritance, encapsulation (private and protected states), static properties, and polymorphism (JS is the polymorphic king) 

Style and code

I can't help but feel there is a lot missing from your code.

Problems I see


It drops requests if they timeout but only when a new request is added by the same client. The object ignores all existing client requests if the client makes no more requests.
You have a const MAX_REQUESTS_IN_TIME_FRAME but the code keeps on adding requests irrespective of this value when addRequestAndCheckIsAllowed is called.
There is no easy way to know that the limit has been reached without creating and adding a new request. To me it seems like you may use the two objects to deduce this state, if so that would violate encapsulation rules, as that is for the object to do not external code.


Some points on you code.


Id is an abbreviation and as such is named with a lower case "d"  ie clientId not clientID
Using Array.filter to remove items from an array is easy, but it generates lots of unused arrays for GC to clean up. Splicing items is far more efficient.
It is good to break long lines into multiple lines to stop them going past the right of screen. However doing so for short lines just makes the code harder to read. You have done this in 3 places for lines that fit even the stack's snippets thin format. More thought on good names can help reduce line lengths.
addRequestAndCheckIsAllowed that is a mouth full, maybe addRequestValidate is a better option?


OO points


Underscore does not make a private variable, avoid its use as its just noise
The constants MAX_REQUESTS_IN_TIME_FRAME and REQUEST_MAX_AGE also are public, and just add to the global scope. Put them inside the objects that use them.
RequestBuffer is only used by RateLimiter, it should not be visible outside RateLimiter
RequestBuffer need only expose a way to store requests and a way to see if a request is valid. This can be done via a getter allowed and a setter store
RequestLimiter should only expose the function addRequestAndCheckIsAllowed all the rest should be protected as private functions and properties.
You create a client request ad hoc, should be an object created via a function. Better yet as it is strongly associated with the RequestLimiter you can add it as a static such as RequestLimiter.Request In the example I add it using define property to ensure it is protected


Encapsualtion and private state

An important part of OO code is that you can protect the object's state by limiting access. In many languages this is done by declaring private variables and methods. Javascript does have a way to define private objects properties but it is very new, and personally because it adds a prefix to the variable name I can not agree with its use. There is a better way.

If you use function syntax to define objects. You can protect state via closure. You can protect object properties using Object.freeze or use Object.defineProperty. You can add statics directly to the function or via Object.defineProperty to protect them.

Getters and Setters

Additionally to help protect an object's state, objects use functions to access properties. These are called getters and setters and allow the objects to ensure only valid states can be set, or read. It also allows you to easily define read or write only properties.

Rewrite

The rewrite uses function syntax to create the same objects as your code. It includes examples of a getter, a setter, a static and private properties and objects via closure. I renamed RequestBuffer to just Buffer as it is inferred by context what it holds and the name no longer lives in the global scope.

The code feels like its missing functionality, but I can only go by what you have supplied.

function RateLimiter() {
    const buffers = {}, MAX_IN_TIME_FRAME = 100, MAX_AGE = 1000;
    function Buffer(maxAge = MAX_AGE) {
        const requests = [];
        function update() { 
            var i = requests.length;
            const maxTime = Date.now() - maxAge;            
            while (i--) {
                if (requests[i].time &gt;= maxTime) {  requests.splice(i, 1) }
            }
        }
        return {
            set store(request) {
                requests.push(request);
                update();
            },
            get allowed() { return requests.length &lt;= MAX_IN_TIME_FRAME }
        };
    }
    function addClient(id) {
        if (buffers[id]) { return buffers[id] }
        return buffers[id] = Buffer();
    }
    return Object.freeze({   // freeze the interface to protect state 
        addRequestValidate(request) {
            const buffer = addClient(request.clientId);
            buffer.store = request;
            return buffer.allowed;
        },
        canClientRequest(id) { return buffers[id] ? buffers[id].allowed : true }
    });
}
Object.defineProperty(RateLimiter, 'Request', { // Request constructor as a static of RateLimiter
    enumerable: false, configurable: false, writable: false,
    value: function(clientId) { return { clientId, time: Date.now() } }
});


Using function syntax to define object makes the new token optional.

const rateLimiter = new RateLimiter();  
//const rateLimiter = RateLimiter();//  works the same

const request = RateLimiter.Request(1);
// or 
// const request = new RateLimiter.Request(1);

const isRequestAllowed = rateLimiter.addRequestValidate(request);


The state of this

I personally believe that good OO JavaScript rarely uses the this token. You will notice that this is not used at all in the above code.

The token this can be so ubiquitous that it occurs multiple times on almost every line of code, it becomes noise, detracting from the overall readability. You used the token 'this' 15 times and your code contains 15 lines of expressions and statements.

More importantly this is JavaScript's achilles heel as it can be easily hijacked. Many people do this as standard practice via Function.call. This means that no matter how much effort you put into protecting an object's state you can never fully trust the state of this

Example using Function.call to change object's state



class ObjA1 {
    constructor() { this.a = "A" }
    log() { console.log(this.a) }
}
const a = new ObjA1();
a.log(); // State is correct

// Hijacking the state.
a.log.call({get a(){console.log("Hijacked and running external code"); return "Boo"; }});  




What is even more worrying is if you have code running on a public page (ie you have 3rd party advertising scripts) this can be hijacked by malicious 3rd parties and used to gain access to secure content. 

The class syntax encourages you to use the token this while function syntax is more conducive to defining a trusted named state.

function ObjA1() {
     const obj = {  // this is replaced by obj
         a : "A",
         log() { console.log(obj.a) }
     }
     return obj;
}