Skip to content

Commit f54a8a7

Browse files
benzekrimahabenzekrimaha
committed
Code and tests update
1 parent 675ecb0 commit f54a8a7

File tree

3 files changed

+238
-7
lines changed

3 files changed

+238
-7
lines changed

‎CountItems/CountWorker.js‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const { BucketInfo } = require('arsenal').models;
44
const monitoring = require('../utils/monitoring');
55
const { deserializeBigInts, serializeBigInts } = require('./utils/utils');
66

7+
const PENSIEVE = 'PENSIEVE';
78
class CountWorker {
89
constructor(params) {
910
this.log = params.log;
@@ -27,6 +28,40 @@ class CountWorker {
2728
return this.client.setup(callback);
2829
}
2930

31+
getIsTransient(bucketInfo, cb) {
32+
const locConstraint = bucketInfo.getLocationConstraint();
33+
34+
if (this.client.isLocationTransient) {
35+
this.client.isLocationTransient(locConstraint, this.log, cb);
36+
return;
37+
}
38+
this.pensieveLocationIsTransient(locConstraint, cb);
39+
}
40+
41+
pensieveLocationIsTransient(locConstraint, cb) {
42+
const overlayVersionId = 'configuration/overlay-version';
43+
44+
async.waterfall([
45+
next => this.client.getObject(PENSIEVE, overlayVersionId, null, this.log, next),
46+
(version, next) => {
47+
const overlayConfigId = `configuration/overlay/${version}`;
48+
return this.client.getObject(PENSIEVE, overlayConfigId, null, this.log, next);
49+
},
50+
], (err, res) => {
51+
if (err) {
52+
this.log.error('error getting configuration overlay', {
53+
method: 'pensieveLocationIsTransient',
54+
error: err,
55+
});
56+
return cb(err);
57+
}
58+
const isTransient =
59+
Boolean(res?.locations[locConstraint]?.isTransient);
60+
61+
return cb(null, isTransient);
62+
});
63+
}
64+
3065
countItems(bucketInfoObj, callback) {
3166
if (!this.client.client) {
3267
return callback(new Error('NotConnected'));
@@ -40,7 +75,7 @@ class CountWorker {
4075
const bucketName = bucketInfo.getName();
4176
this.log.info(`${process.pid} handling ${bucketName}`);
4277
return async.waterfall([
43-
next => this.client._getIsTransient(bucketInfo, this.log, next),
78+
next => this.getIsTransient(bucketInfo, next),
4479
(isTransient, next) => this.client.getObjectMDStats(bucketName, bucketInfo, isTransient, this.log, next),
4580
], (err, results) => {
4681
monitoring.bucketsCount.inc({ status: err ? 'error' : 'success' });

‎tests/mocks/mongoClient.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,9 @@ module.exports = {
44
},
55
setup: jest.fn(),
66
close: jest.fn(),
7-
_getIsTransient: jest.fn(),
87
getObjectMDStats: jest.fn(),
8+
isLocationTransient: jest.fn(),
9+
getObject: jest.fn(),
910
getBucketInfos: jest.fn(),
1011
updateStorageConsumptionMetrics: jest.fn(),
1112
getUsersBucketCreationDate: jest.fn(),

‎tests/unit/CountItems/CountWorker.js‎

Lines changed: 200 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ describe('CountItems::CountWorker', () => {
1010
mongoMock.close.mockReset();
1111
mongoMock.setup.mockReset();
1212
mongoMock.client.isConnected.mockReset();
13-
mongoMock._getIsTransient.mockReset();
1413
mongoMock.getObjectMDStats.mockReset();
14+
mongoMock.getObject.mockReset();
1515
});
1616

1717
const t = [
@@ -180,13 +180,11 @@ describe('CountItems::CountWorker', () => {
180180
sendFn: testSendFn,
181181
client: mongoMock,
182182
});
183+
w.getIsTransient = jest.fn((bucketInfo, cb) => cb(...tc.mock.getIsTransient));
183184
mongoMock.setup.mockImplementationOnce(cb => cb(...tc.mock.setup));
184185
mongoMock.close.mockImplementationOnce(cb => cb(...tc.mock.close));
185186
mongoMock.client.isConnected
186187
.mockImplementationOnce(() => tc.mock.isConnected);
187-
mongoMock._getIsTransient.mockImplementationOnce(
188-
(_a, _b, cb) => cb(...tc.mock.getIsTransient),
189-
);
190188
mongoMock.getObjectMDStats.mockImplementationOnce(
191189
(_a, _b, _c, _d, cb) => cb(...tc.mock.getObjectMDStats),
192190
);
@@ -213,15 +211,212 @@ describe('CountItems::CountWorker', () => {
213211
_creationDate: Date.now().toString(),
214212
website: { indexDocument: 'index.html' },
215213
};
214+
w.getIsTransient = jest.fn((bucketInfo, cb) => cb(null, true));
216215
mongoMock.setup.mockImplementationOnce(cb => cb());
217216
mongoMock.close.mockImplementationOnce(cb => cb());
218217
mongoMock.client.isConnected.mockImplementationOnce(() => false);
219-
mongoMock._getIsTransient.mockImplementationOnce((_a, _b, cb) => cb(null, true));
220218
mongoMock.getObjectMDStats.mockImplementationOnce((_a, _b, _c, _d, cb) => cb(null, { value: 42 }));
221219
w.countItems(bucketInfo, (err, results) => {
222220
expect(err).toBeNull();
223221
expect(results).toEqual({ value: 42 });
224222
done();
225223
});
226224
});
225+
226+
describe('CountWorker.getIsTransient method', () => {
227+
let worker;
228+
let mockBucketInfo;
229+
230+
beforeEach(() => {
231+
worker = new CountWorker({
232+
log: new DummyLogger(),
233+
sendFn: jest.fn(),
234+
client: mongoMock,
235+
});
236+
mockBucketInfo = {
237+
_name: 'test-bucket',
238+
_owner: 'any',
239+
_ownerDisplayName: 'any',
240+
_creationDate: Date.now().toString(),
241+
getLocationConstraint: jest.fn(() => 'test-location'),
242+
};
243+
});
244+
245+
test('should use client.isLocationTransient if available and return true', done => {
246+
mongoMock.isLocationTransient = jest.fn((loc, lg, cb) => cb(null, true));
247+
worker.getIsTransient(mockBucketInfo, (err, isTransient) => {
248+
expect(err).toBeNull();
249+
expect(isTransient).toBe(true);
250+
expect(mongoMock.isLocationTransient).toHaveBeenCalledWith('test-location', worker.log, expect.any(Function));
251+
done();
252+
});
253+
});
254+
255+
test('should use client.isLocationTransient if available and return false', done => {
256+
mongoMock.isLocationTransient = jest.fn((loc, lg, cb) => cb(null, false));
257+
worker.getIsTransient(mockBucketInfo, (err, isTransient) => {
258+
expect(err).toBeNull();
259+
expect(isTransient).toBe(false);
260+
done();
261+
});
262+
});
263+
264+
test('should propagate error from client.isLocationTransient', done => {
265+
const testError = new Error('client.isLocationTransient error');
266+
mongoMock.isLocationTransient = jest.fn((loc, lg, cb) => cb(testError));
267+
worker.getIsTransient(mockBucketInfo, (err, isTransient) => {
268+
expect(err).toBe(testError);
269+
expect(isTransient).toBeUndefined();
270+
done();
271+
});
272+
});
273+
274+
describe('when client.isLocationTransient is not available (fallback to pensieve)', () => {
275+
beforeEach(() => {
276+
mongoMock.isLocationTransient = undefined;
277+
});
278+
279+
test('should fallback to pensieveLocationIsTransient and return true', done => {
280+
mongoMock.getObject
281+
.mockImplementationOnce((bucket, key, params, log, cb) => cb(null, 'v1')) // 1st call
282+
.mockImplementationOnce((bucket, key, params, log, cb) => cb(null, { // 2nd call
283+
locations: { 'test-location': { isTransient: true } },
284+
}));
285+
worker.getIsTransient(mockBucketInfo, (err, isTransient) => {
286+
expect(err).toBeNull();
287+
expect(isTransient).toBe(true);
288+
expect(mongoMock.getObject).toHaveBeenCalledTimes(2);
289+
done();
290+
});
291+
});
292+
293+
test('should fallback to pensieveLocationIsTransient and return false', done => {
294+
mongoMock.getObject
295+
.mockImplementationOnce((bucket, key, params, log, cb) => cb(null, 'v1'))
296+
.mockImplementationOnce((bucket, key, params, log, cb) => cb(null, {
297+
locations: { 'test-location': { isTransient: false } },
298+
}));
299+
worker.getIsTransient(mockBucketInfo, (err, isTransient) => {
300+
expect(err).toBeNull();
301+
expect(isTransient).toBe(false);
302+
done();
303+
});
304+
});
305+
306+
test('should propagate error from pensieveLocationIsTransient (e.g., first getObject fails)', done => {
307+
const pensieveError = new Error('Pensieve getObject error');
308+
mongoMock.getObject.mockImplementationOnce((b, k, p, l, cb) => cb(pensieveError));
309+
worker.getIsTransient(mockBucketInfo, (err, isTransient) => {
310+
expect(err).toBe(pensieveError);
311+
expect(isTransient).toBeUndefined();
312+
done();
313+
});
314+
});
315+
});
316+
});
317+
318+
describe('CountWorker.pensieveLocationIsTransient method', () => {
319+
let worker;
320+
const PENSIEVE_BUCKET_NAME = 'PENSIEVE';
321+
322+
beforeEach(() => {
323+
worker = new CountWorker({
324+
log: new DummyLogger(),
325+
sendFn: jest.fn(),
326+
client: mongoMock,
327+
});
328+
});
329+
330+
test('should return true if location is transient in Pensieve config', done => {
331+
mongoMock.getObject
332+
.mockImplementationOnce((bucket, key, params, log, cb) => {
333+
expect(bucket).toBe(PENSIEVE_BUCKET_NAME);
334+
expect(key).toBe('configuration/overlay-version');
335+
cb(null, 'v-p-1');
336+
})
337+
.mockImplementationOnce((bucket, key, params, log, cb) => {
338+
expect(bucket).toBe(PENSIEVE_BUCKET_NAME);
339+
expect(key).toBe('configuration/overlay/v-p-1');
340+
cb(null, { locations: { 'loc-A': { isTransient: true } } });
341+
});
342+
343+
worker.pensieveLocationIsTransient('loc-A', (err, isTransient) => {
344+
expect(err).toBeNull();
345+
expect(isTransient).toBe(true);
346+
done();
347+
});
348+
});
349+
350+
test('should return false if location is not transient in Pensieve config', done => {
351+
mongoMock.getObject
352+
.mockImplementationOnce((b, k, p, l, cb) => cb(null, 'v-p-2'))
353+
.mockImplementationOnce((b, k, p, l, cb) => cb(null, { locations: { 'loc-B': { isTransient: false } } }));
354+
355+
worker.pensieveLocationIsTransient('loc-B', (err, isTransient) => {
356+
expect(err).toBeNull();
357+
expect(isTransient).toBe(false);
358+
done();
359+
});
360+
});
361+
362+
test('should return false if isTransient property is missing', done => {
363+
mongoMock.getObject
364+
.mockImplementationOnce((b, k, p, l, cb) => cb(null, 'v-p-3'))
365+
.mockImplementationOnce((b, k, p, l, cb) => cb(null, { locations: { 'loc-C': {} } }));
366+
367+
worker.pensieveLocationIsTransient('loc-C', (err, isTransient) => {
368+
expect(err).toBeNull();
369+
expect(isTransient).toBe(false);
370+
done();
371+
});
372+
});
373+
374+
test('should propagate error from first getObject call', done => {
375+
const testError = new Error('getObject overlay-version failed');
376+
mongoMock.getObject.mockImplementationOnce((b, k, p, l, cb) => cb(testError));
377+
378+
worker.pensieveLocationIsTransient('loc-D', (err, isTransient) => {
379+
expect(err).toBe(testError);
380+
expect(isTransient).toBeUndefined();
381+
done();
382+
});
383+
});
384+
385+
test('should propagate error from second getObject call', done => {
386+
const testError = new Error('getObject overlay-config failed');
387+
mongoMock.getObject
388+
.mockImplementationOnce((b, k, p, l, cb) => cb(null, 'v-p-4'))
389+
.mockImplementationOnce((b, k, p, l, cb) => cb(testError));
390+
391+
worker.pensieveLocationIsTransient('loc-E', (err, isTransient) => {
392+
expect(err).toBe(testError);
393+
expect(isTransient).toBeUndefined();
394+
done();
395+
});
396+
});
397+
398+
test('should handle case when locConstraint is not in locations', done => {
399+
mongoMock.getObject
400+
.mockImplementationOnce((b, k, p, l, cb) => cb(null, 'v-p-5'))
401+
.mockImplementationOnce((b, k, p, l, cb) => cb(null, { locations: { 'other-loc': { isTransient: true } } }));
402+
403+
worker.pensieveLocationIsTransient('loc-F-not-in-response', (err, isTransient) => {
404+
expect(err).toBeNull();
405+
expect(isTransient).toBe(false);
406+
done();
407+
});
408+
});
409+
410+
test('should handle case where no location is provided', done => {
411+
mongoMock.getObject
412+
.mockImplementationOnce((b, k, p, l, cb) => cb(null, 'v-p-6'))
413+
.mockImplementationOnce((b, k, p, l, cb) => cb(null, { locations: {} }));
414+
415+
worker.pensieveLocationIsTransient('loc-G', (err, isTransient) => {
416+
expect(err).toBeNull();
417+
expect(isTransient).toBe(false);
418+
done();
419+
});
420+
});
421+
});
227422
});

0 commit comments

Comments
 (0)