Skip to content

Commit

Permalink
refactor: remove all methods that accept a $id
Browse files Browse the repository at this point in the history 
BREAKING CHANGE:

All methods ending with "ById" are removed but one.
getEntryById remains but only returns something if the user requesting the document is its original owner ($owners[0])
  • Loading branch information
@targos
targos committed Nov 14, 2016
1 parent 2646dd0 commit a141dec
Show file tree
Hide file tree
Showing 14 changed files with 106 additions and 179 deletions.
18 changes: 1 addition & 17 deletions src/couch/attachment.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,6 @@ const nanoPromise = require('../util/nanoPromise');
const nanoMethods = require('./nano');

const methods = {
async addAttachmentsById(id, user, attachments) {
debug(`addAttachmentsById (${id}, ${user})`);
if (!Array.isArray(attachments)) {
attachments = [attachments];
}
const entry = await this.getEntryByIdAndRights(id, user, ['write', 'addAttachment']);
return nanoPromise.attachFiles(this._db, entry, attachments);
},

async addAttachmentsByUuid(uuid, user, attachments) {
debug(`addAttachmentsByUuid (${uuid}, ${user})`);
if (!Array.isArray(attachments)) {
Expand All @@ -36,12 +27,6 @@ const methods = {
return nanoMethods.saveEntry(this._db, entry, user);
},

async getAttachmentByIdAndName(id, name, user, asStream, options) {
debug(`getAttachmentByIdAndName (${id}, ${name}, ${user})`);
const entry = await this.getEntryById(id, user, options);
return getAttachmentFromEntry(entry, this, name, asStream);
},

async getAttachmentByUuidAndName(uuid, name, user, asStream, options) {
debug(`getAttachmentByUuidAndName (${uuid}, ${name}, ${user})`);
const entry = await this.getEntryByUuid(uuid, user, options);
Expand All @@ -62,7 +47,7 @@ const methods = {
throw new CouchError('file must have field, name and data properties');
}

const entry = await this.getEntryByIdAndRights(id, user, ['write']);
const entry = await this.getEntryById(id, user);
let current = entry.$content || {};

if (newContent) {
Expand Down Expand Up @@ -111,7 +96,6 @@ const methods = {
}
};

methods.addAttachmentById = methods.addAttachmentsById;
methods.addAttachmentByUuid = methods.addAttachmentsByUuid;

async function getAttachmentFromEntry(entry, ctx, name, asStream) {
Expand Down
31 changes: 11 additions & 20 deletions src/couch/doc.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,51 +11,42 @@ const nanoMethods = require('./nano');
const ensureStringArray = require('../util/ensureStringArray');

const methods = {
async getDocByRights(idOrUuid, user, rights, options) {
async getDocByRights(uuid, user, rights, type, options) {
await this.open();
debug.trace('getDocByRights');
if (!util.isManagedDocumentType(options.type)) {
throw new CouchError(`invalid document type: ${options.type}`);
if (!util.isManagedDocumentType(type)) {
throw new CouchError(`invalid type argument: ${type}`);
}

let uuid;
if (idOrUuid.id) {
uuid = await nanoMethods.getUuidFromId(this._db, idOrUuid.id, options.type, user);
} else if (idOrUuid.uuid) {
uuid = idOrUuid.uuid;
} else {
throw new CouchError('missing id or uuid');
}


const doc = await nanoPromise.getDocument(this._db, uuid);
if (!doc) {
throw new CouchError('document not found', 'not found');
}
if (doc.$type !== options.type) {
throw new CouchError(`wrong document type: ${doc.$type}. Expected: ${options.type}`);
if (doc.$type !== type) {
throw new CouchError(`wrong document type: ${doc.$type}. Expected: ${type}`);
}

if (await validate.validateTokenOrRights(this._db, uuid, doc.$owners, rights, user, options.token, options.type)) {
const token = options ? options.token : null;
if (await validate.validateTokenOrRights(this._db, uuid, doc.$owners, rights, user, token, type)) {
return nanoPromise.getDocument(this._db, uuid, options);
}
throw new CouchError('user has no access', 'unauthorized');
},

async addOwnersToDoc(idOrUuid, user, owners, options) {
async addOwnersToDoc(uuid, user, owners, type, options) {
await this.open();
debug.trace('addOwnersToDoc');
owners = ensureOwnersArray(owners);
const doc = await this.getDocByRights(idOrUuid, user, 'owner', options);
const doc = await this.getDocByRights(uuid, user, 'owner', type, options);
doc.$owners = _.union(doc.$owners, owners);
return nanoMethods.save(this._db, doc, user);
},

async removeOwnersFromDoc(idOrUuid, user, owners, options) {
async removeOwnersFromDoc(uuid, user, owners, type, options) {
await this.open();
debug.trace('removeOwnersFromDoc');
owners = ensureOwnersArray(owners);
const doc = await this.getDocByRights(idOrUuid, user, 'owner', options);
const doc = await this.getDocByRights(uuid, user, 'owner', type, options);
const newArray = _.pullAll(doc.$owners.slice(1), owners);
newArray.unshift(doc.$owners[0]);
doc.$owners = newArray;
Expand Down
74 changes: 21 additions & 53 deletions src/couch/entry.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,6 @@ const nanoMethods = require('./nano');
const util = require('./util');

const methods = {
async getEntryByIdAndRights(id, user, rights, options = {}) {
debug(`getEntryByIdAndRights (${id}, ${user}, ${rights})`);
await this.open();

const owners = await getOwnersById(this._db, id);
if (owners.length === 0) {
debug.trace(`no entry matching id ${id}`);
throw new CouchError('document not found', 'not found');
}
let hisEntry = owners.find(own => own.value[0] === user);
if (!hisEntry) {
hisEntry = owners[0];
}

if (await validateMethods.validateTokenOrRights(this._db, hisEntry.id, hisEntry.value, rights, user, options.token)) {
debug.trace(`user ${user} has access`);
return nanoPromise.getDocument(this._db, hisEntry.id, options);
}

debug.trace(`user ${user} has no ${rights} access`);
throw new CouchError('user has no access', 'unauthorized');
},

async getEntryByUuidAndRights(uuid, user, rights, options = {}) {
debug(`getEntryByUuidAndRights (${uuid}, ${user}, ${rights})`);
await this.open();
Expand Down Expand Up @@ -63,8 +40,12 @@ const methods = {
return this.getEntryByUuidAndRights(uuid, user, 'read', options);
},

getEntryById(id, user, options) {
return this.getEntryByIdAndRights(id, user, 'read', options);
// this function can only return an entry for its main owner
async getEntryById(id, user, options) {
await this.open();
debug(`getEntryById (${id}, ${user})`);
const uuid = await nanoMethods.getUuidFromId(this._db, id, 'entry', user);
return this.getDocByRights(uuid, user, 'owner', 'entry', options);
},

async deleteEntryByUuid(uuid, user) {
Expand All @@ -73,12 +54,6 @@ const methods = {
return nanoPromise.destroyDocument(this._db, uuid);
},

async deleteEntryById(id, user) {
debug(`deleteEntryById (${id}, ${user})`);
const doc = await this.getEntryByIdAndRights(id, user, 'delete');
return nanoPromise.destroyDocument(this._db, doc._id);
},

async createEntry(id, user, options) {
options = options || {};
debug(`createEntry (id: ${id}, user: ${user}, kind: ${options.kind})`);
Expand Down Expand Up @@ -152,17 +127,6 @@ const methods = {
return Promise.all(allowedDocs.map(doc => nanoPromise.getDocument(this._db, doc.id)));
},

async _doUpdateOnEntry(id, user, update, updateBody) {
// Call update handler
await this.open();
const doc = await this.getEntryById(id, user);
const hasRight = validateMethods.isOwner(doc.$owners, user);
if (!hasRight) {
throw new CouchError('unauthorized to edit group (only owner can)', 'unauthorized');
}
return nanoPromise.updateWithHandler(this._db, update, doc._id, updateBody);
},

async _doUpdateOnEntryByUuid(uuid, user, update, updateBody) {
await this.open();
const doc = await this.getEntryByUuid(uuid, user);
Expand Down Expand Up @@ -196,17 +160,25 @@ const methods = {
const doc = await this.getEntryByUuidAndRights(entry._id, user, ['write']);
result = await updateEntry(this, doc, entry, user, options);
} catch (e) {
result = await notFound(e);
action = 'created';
if (e.reason === 'not found') {
result = await notFound(e);
action = 'created';
} else {
throw e;
}
}
} else if (entry.$id) {
debug.trace('entry has no _id but has $id');
try {
const doc = await this.getEntryByIdAndRights(entry.$id, user, ['write']);
result = await updateEntry(this, doc, entry, user, options);
await this.getEntryById(entry.$id, user);
throw new CouchError('entry already exists', 'conflict');
} catch (e) {
result = await notFound(e);
action = 'created';
if (e.reason === 'not found') {
result = await notFound(e);
action = 'created';
} else {
throw e;
}
}
} else {
debug.trace('entry has no _id nor $id');
Expand All @@ -223,12 +195,8 @@ const methods = {
}
};

async function getOwnersById(db, id) {
return nanoPromise.queryView(db, 'ownersById', {key: id});
}

function onNotFound(ctx, entry, user, options) {
return async (error) => {
return async(error) => {
if (error.reason === 'not found') {
debug.trace('doc not found');
if (options.isUpdate) {
Expand Down
10 changes: 0 additions & 10 deletions src/couch/group.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,21 +49,11 @@ const methods = {
return this.editDefaultGroup(group, type, 'remove');
},

addGroupToEntry(id, user, group) {
debug(`addGroupToEntry (${id}, ${user}, ${group})`);
return this._doUpdateOnEntry(id, user, 'addGroupToEntry', {group});
},

addGroupToEntryByUuid(uuid, user, group) {
debug(`addGroupToEntryByUuid (${uuid}, ${user}, ${group})`);
return this._doUpdateOnEntryByUuid(uuid, user, 'addGroupToEntry', {group});
},

removeGroupFromEntry(id, user, group) {
debug(`removeGroupFromEntry (${id}, ${user}, ${group})`);
return this._doUpdateOnEntry(id, user, 'removeGroupFromEntry', {group});
},

removeGroupFromEntryByUuid(uuid, user, group) {
debug(`removeGroupFromEntryByUuid (${uuid}, ${user}, ${group})`);
return this._doUpdateOnEntryByUuid(uuid, user, 'removeGroupFromEntry', {group});
Expand Down
4 changes: 0 additions & 4 deletions src/server/middleware/couch.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,6 @@ exports.saveAttachment = composeWithError(function*() {
});
});

exports.getAttachmentById = composeWithError(function*() {
this.body = yield this.state.couch.getAttachmentByIdAndName(this.params.id, this.params.attachment, this.state.userEmail, true, this.query);
});

exports.getAttachmentByUuid = composeWithError(function*() {
this.body = yield this.state.couch.getAttachmentByUuidAndName(this.params.uuid, this.params.attachment, this.state.userEmail, true, this.query);
});
Expand Down
18 changes: 6 additions & 12 deletions test/attachments.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,33 +6,27 @@ describe('entries with attachments', function () {
before(data);

it('should error if entry has no attachment', function () {
return couch.getAttachmentByIdAndName('anonymousEntry', 'foo.txt', 'b@b.com')
return couch.getAttachmentByUuidAndName('anonymousEntry', 'foo.txt', 'b@b.com')
.should.be.rejectedWith(/attachment foo\.txt not found/);
});

it('should error if entry attachment does not exist', function () {
return couch.getAttachmentByIdAndName('entryWithAttachment', 'foo.txt', 'b@b.com')
return couch.getAttachmentByUuidAndName('entryWithAttachment', 'foo.txt', 'b@b.com')
.should.be.rejectedWith(/attachment foo\.txt not found/);
});

it('should return attachment data from id', function () {
return couch.getAttachmentByIdAndName('entryWithAttachment', 'test.txt', 'b@b.com')
.should.be.fulfilledWith(new Buffer('THIS IS A TEST'));
});

it('should return attachment data from uuid', function () {
return couch.getEntryById('entryWithAttachment', 'b@b.com')
.then(entry => couch.getAttachmentByUuidAndName(entry._id, 'test.txt', 'b@b.com'))
it('should return attachment data', function () {
return couch.getAttachmentByUuidAndName('entryWithAttachment', 'test.txt', 'b@b.com')
.should.be.fulfilledWith(new Buffer('THIS IS A TEST'));
});

it('should delete an attachment from a document given by its uuid', function () {
return couch.getEntryById('entryWithAttachment', 'b@b.com')
return couch.getEntryByUuid('entryWithAttachment', 'b@b.com')
.then(entry => couch.deleteAttachmentByUuid(entry._id, 'b@b.com', 'test.txt', {
rev: entry._rev
}))
.then(() => {
return couch.getAttachmentByIdAndName('entryWithAttachment', 'test.txt', 'b@b.com')
return couch.getAttachmentByUuidAndName('entryWithAttachment', 'test.txt', 'b@b.com')
.should.be.rejectedWith(/attachment test\.txt not found/);
});
});
Expand Down
1 change: 1 addition & 0 deletions test/data/anyuser.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ function populate(db) {
$type: 'entry',
$owners: ['b@b.com', 'groupA', 'groupB'],
$id: 'A',
_id: 'A',
$creationDate: 0,
$modificationDate: 0,
$content: {}
Expand Down
4 changes: 3 additions & 1 deletion test/data/constants.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ const constants = {
},
newEntryWithId: {
$id: 'D',
$content: {test: true},
$content: {
test: true
},
_id: 'D'
}
};
Expand Down
4 changes: 4 additions & 0 deletions test/data/data.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ function populate(db) {
$type: 'group',
$owners: ['a@a.com'],
name: 'groupA',
_id: 'groupA',
users: ['a@a.com'],
rights: ['create', 'write', 'delete', 'read']
}));
Expand All @@ -25,6 +26,7 @@ function populate(db) {
$type: 'group',
$owners: ['a@a.com'],
name: 'groupB',
_id: 'groupB',
users: ['a@a.com'],
rights: ['create']
}));
Expand Down Expand Up @@ -71,6 +73,7 @@ function populate(db) {
$type: 'entry',
$owners: ['b@b.com', 'groupC'],
$id: 'anonymousEntry',
_id: 'anonymousEntry',
$creationDate: 0,
$modificationDate: 0
}));
Expand All @@ -79,6 +82,7 @@ function populate(db) {
$type: 'entry',
$owners: ['c@c.com'],
$id: 'entryWithAttachment',
_id: 'entryWithAttachment',
$creationDate: 0,
$modificationDate: 0,
_attachments: {
Expand Down
Loading

0 comments on commit a141dec

Please sign in to comment.