Skip to content

Commit fcd8233

Browse files
committed
Implement object merge logic for updateBucket
By mixing in both the existing data as well as the incoming patch data, we can check if the new combination still yields a valid and reachable bucket. Signed-off-by: Jeremy Ho <jujaga@gmail.com>
1 parent bc388ce commit fcd8233

File tree

2 files changed

+14
-8
lines changed

2 files changed

+14
-8
lines changed

app/src/controllers/bucket.js

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -215,13 +215,17 @@ const controller = {
215215
* @returns {function} Express middleware function
216216
*/
217217
async updateBucket(req, res, next) {
218-
// Check for credential accessibility/validity first
219-
await controller._validateCredentials(req.body, res);
220-
221218
try {
219+
const bucketId = addDashesToUuid(req.params.bucketId);
220+
const currentBucket = await bucketService.read(bucketId);
221+
222+
// Check for credential accessibility/validity first
223+
// Need to cross reference with existing data when partial patch data is provided
224+
await controller._validateCredentials({ ...currentBucket, ...req.body });
225+
222226
const userId = await userService.getCurrentUserId(getCurrentIdentity(req.currentUser, SYSTEM_USER), SYSTEM_USER);
223227
const response = await bucketService.update({
224-
bucketId: req.params.bucketId,
228+
bucketId: bucketId,
225229
bucketName: req.body.bucketName,
226230
accessKeyId: req.body.accessKeyId,
227231
bucket: req.body.bucket,

app/tests/unit/controllers/bucket.spec.js

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ describe('createBucket', () => {
107107
);
108108
});
109109

110-
it('excepts if invalid bucket head', async () => {
110+
it('responds with error when invalid bucket head', async () => {
111111
// request object
112112
const req = {
113113
body: { bucket: REQUEST_BUCKET, region: 'test' },
@@ -126,13 +126,15 @@ describe('createBucket', () => {
126126
throw new Error();
127127
});
128128

129-
await expect(controller.createBucket(req, res, next)).rejects.toThrow();
129+
await controller.createBucket(req, res, next);
130130

131131
expect(headBucketSpy).toHaveBeenCalledTimes(1);
132132
expect(headBucketSpy).toHaveBeenCalledWith(req.body);
133133
expect(getCurrentIdentitySpy).toHaveBeenCalledTimes(0);
134134
expect(getCurrentUserIdSpy).toHaveBeenCalledTimes(0);
135135
expect(createSpy).toHaveBeenCalledTimes(0);
136+
expect(next).toHaveBeenCalledTimes(1);
137+
expect(next).toHaveBeenCalledWith(expect.any(Problem));
136138
});
137139

138140
it('nexts an error if the bucket service fails to create', async () => {
@@ -167,7 +169,7 @@ describe('createBucket', () => {
167169
expect(getCurrentUserIdSpy).toHaveBeenCalledWith(USR_IDENTITY);
168170
expect(createSpy).toHaveBeenCalledTimes(1);
169171
expect(createSpy).toHaveBeenCalledWith({ ...req.body, userId: USR_ID });
170-
172+
171173
expect(next).toHaveBeenCalledTimes(1);
172174
expect(next).toHaveBeenCalledWith(
173175
new Problem(502, 'Unknown BucketService Error')
@@ -213,7 +215,7 @@ describe('createBucket', () => {
213215
expect(createSpy).toHaveBeenCalledWith({ ...req.body, userId: USR_ID });
214216
expect(checkGrantPermissionsSpy).toHaveBeenCalledTimes(1);
215217
expect(checkGrantPermissionsSpy).toHaveBeenCalledWith({ ...req.body, userId: USR_ID });
216-
218+
217219
expect(res.status).toHaveBeenCalledWith(201);
218220
});
219221
});

0 commit comments

Comments
 (0)