Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

Feature/sc 3033 reconnect #50

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 18 additions & 18 deletions config/default.json
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
{
"host": "localhost",
"port": 4001,
"protocol": "http",
"mongodb": "mongodb://localhost:27017/schulcloud-editor",
"routes": {
"server": {
"baseURL": "http://localhost:3030",
"coursePermissionsUri": "/courses/:courseId/userPermissions",
"meUri": "/me",
"courseMembersUri": "/courses/:courseId/members"
},
"timeout": "30000"
},
"testsecret": "secret",
"redis": "REDIS_URI",
"redis_key": "REDIS_KEY"
}
{
Copy link
Contributor

@CeEv CeEv Mar 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you set this file to default please.

"host": "api.edtr.l",
"port": 4001,
"protocol": "http",
"mongodb": "mongodb://mongodb:27017/schulcloud-editor",
"routes": {
"server": {
"baseURL": "http://server:3030",
"coursePermissionsUri": "/courses/:courseId/userPermissions",
"meUri": "/me",
"courseMembersUri": "/courses/:courseId/members"
},
"timeout": "30000"
},
"testsecret": "secret",
"redis": "REDIS_URI",
"redis_key": "REDIS_KEY"
}
2 changes: 1 addition & 1 deletion src/global/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ exports.before = {
const after = {
// todo select is better but need more stable implementations
all: [iff(isProvider('external'),
filterOutResults('__v', 'createdAt', 'updatedAt')),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this? @CordlessWool

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its a long time ago, I could not remember, but its filter out before sending it to the client? For the client the data are not necassery

filterOutResults('__v', 'createdBy', 'updatedBy')),
],
find: [],
get: [],
Expand Down
3 changes: 2 additions & 1 deletion src/middleware/socket.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ module.exports = app => app.configure(
socketio((io) => {
io.on('connection', (socket) => {
// do some authorization things
app.channel('anonymous').join(socket.feathers);
// app.channel('anonymous').join(socket.feathers);
Comment on lines 7 to +8
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this be removed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

der Change ist von @CordlessWool , gebe die Frage weiter :)

Copy link
Contributor

@CeEv CeEv Mar 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ich glaube er wollte ein channel für alle haben. Ich würde es erst mal auskommentiert lassen, bis wir es benötigen.


socket.on('authorization', (data) => {
const { token } = data;
Expand All @@ -14,6 +14,7 @@ module.exports = app => app.configure(
});
});


// socketJwtHandler(io);

io.use((socket, next) => {
Expand Down
2 changes: 2 additions & 0 deletions src/routes/ServiceRoutes.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/* eslint-disable max-classes-per-file */

const axios = require('axios');
const queryString = require('query-string');

Expand Down
1 change: 1 addition & 0 deletions src/services/lessons/models/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const lessonSchema = new Schema({
deletedAt: { type: Date, expires: (60 * 60 * 24 * 30) },
createdBy: { type: Schema.Types.ObjectId },
updatedBy: { type: Schema.Types.ObjectId },
hash: { type: String },
position: { type: Number, default: Date.now },
fork: { type: Schema.Types.ObjectId }, // is forked from
}, {
Expand Down
5 changes: 5 additions & 0 deletions src/services/lessons/schemes/lessons.create.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@
"$id": "#/properties/fork",
"type": "string",
"pattern": "[a-f0-9]{24}"
},
"hash": {
"$id": "#/properties/hash",
"type": "string",
"pattern": "[a-f0-9]{24}"
}
},
"additionalProperties": false
Expand Down
5 changes: 5 additions & 0 deletions src/services/lessons/schemes/lessons.patch.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@
"position": {
"$id": "#/properties/position",
"type": "number"
},
"hash": {
"$id": "#/properties/hash",
"type": "string",
"pattern": "[a-f0-9]{24}"
}
},
"additionalProperties": false
Expand Down
1 change: 1 addition & 0 deletions src/services/sections/models/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const sectionSchema = new Schema({
title: { type: String, default: '' },
state: { type: Object, default: {} },
permissions: [{ type: permissionSchema }],
hash: { type: String },
deletedAt: { type: Date, expires: (60 * 60 * 24 * 30) },
createdBy: { type: Schema.Types.ObjectId, immutable: true },
updatedBy: { type: Schema.Types.ObjectId },
Expand Down
3 changes: 3 additions & 0 deletions src/services/sections/schemas/section.create.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
"state": {
"type": "object"
},
"hash": {
"type": "string"
},
"createdBy": {
"type": "string",
"pattern": "[a-f0-9]{24}"
Expand Down
5 changes: 4 additions & 1 deletion src/services/sections/schemas/section.patch.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
},
"stateDiff": {
"type": "object"
},
},
"hash": {
"type": "string"
},
"updatedBy": {
"type": "string",
"pattern": "[a-f0-9]{24}"
Expand Down
26 changes: 14 additions & 12 deletions src/services/sections/section.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ const {
} = require('../../utils');
const {
diffToMongo,
filterManyByHashes,
filterStateByHash,
} = require('./utils');
const {
create: createSchema,
Expand Down Expand Up @@ -82,9 +84,9 @@ class SectionService {
const internParams = this.setScope(prepareParams(params));
const sections = await this.app.service(this.baseService)
.find(internParams);
const filtered = permissions.filterHasRead(sections.data, params.user);

filtered.map((section) => {
let filtered = permissions.filterHasRead(sections.data, params.user);
filtered = filterManyByHashes(filtered, params.query.hashes);
filtered.forEach((section) => {
section.visible = !permissions.couldAnyoneOnlyRead(section.permissions);
return section;
});
Expand All @@ -93,16 +95,16 @@ class SectionService {
return setUserScopePermissionForFindRequests(sections, params.user);
}

get(id, params) {
async get(id, params) {
const internParams = this.setScope(prepareParams(params));
return this.app.service(this.baseService)
.get(id, internParams)
.then((section) => {
if (!permissions.hasRead(section.permissions, params.user)) {
throw new Forbidden(this.err.noAccess);
}
return setUserScopePermission(section, section.permissions, params.user);
});
const section = await this.app.service(this.baseService).get(id, internParams);
if (!permissions.hasRead(section.permissions, params.user)) {
throw new Forbidden(this.err.noAccess);
}
section.timestamp = section.updatedAt;
const filteredSection = filterStateByHash(section, params.query.hash);
const result = await setUserScopePermission(filteredSection, filteredSection.permissions, params.user);
CeEv marked this conversation as resolved.
Show resolved Hide resolved
return result;
}

async remove(_id, params) {
Expand Down
69 changes: 69 additions & 0 deletions src/services/sections/section.service.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,48 @@ describe('sections/section.service.js', () => {
expect(status).to.equal(403);
});

it('get with hash only sends state if hashes dont match', async () => {
const { _id: lessonId } = await lessonService.createWithDefaultData({ courseId }, readPermission);
const state = { abc: 123, x: [] };
const ref = {
target: lessonId,
targetModel: 'lesson',
};
const hash = 'thisisthecurrenthash';
const section = await sectionService.createWithDefaultData({ state, ref, hash }, readPermission);

expect(lessonId.toString()).to.equal(section.ref.target.toString());

const {
status: matchingHashStatus, data: matchingHashData,
} = await sectionService.sendRequestToThisService('get', {
id: section._id, userId, lessonId, query: { hash: 'thisisthecurrenthash' },
});

expect(matchingHashStatus).to.equal(200);
expect(matchingHashData).to.an('object');
expect(matchingHashData).to.have.property('hash');
expect(matchingHashData).to.not.have.property('ref');
expect(matchingHashData).to.not.have.property('state');
expect(matchingHashData).to.not.have.property('permissions');
expect(matchingHashData).to.have.property(userScopePermissionKey);

const {
status: otherHashStatus, data: otherHashData,
} = await sectionService.sendRequestToThisService('get', {
id: section._id, userId, lessonId, query: { hash: 'someotherhash' },
});

expect(otherHashStatus).to.equal(200);
expect(otherHashData).to.an('object');
expect(otherHashData.ref.target.toString()).to.equal(lessonId.toString());
expect(otherHashData.state).to.deep.equal(state);
expect(otherHashData).to.not.have.property('permissions');
expect(otherHashData).to.have.property(userScopePermissionKey);
expect(otherHashData).to.have.property('timestamp');
expect(otherHashData).to.have.property('hash');
});

it('find with read permissions should work', async () => {
const { _id: lessonId } = await lessonService.createWithDefaultData({ courseId }, writePermission);
const ref = {
Expand All @@ -206,6 +248,33 @@ describe('sections/section.service.js', () => {
expect(data.data).all.have.property(userScopePermissionKey);
});

it('find should filter states if hashes match', async () => {
const { _id: lessonId } = await lessonService.createWithDefaultData({ courseId }, writePermission);
const ref = {
target: lessonId,
targetModel: 'lesson',
};
const sections = await Promise.all([
sectionService.createWithDefaultData({ ref, hash: 'firsthash' }, writePermission),
sectionService.createWithDefaultData({ ref, hash: 'secondhash' }, readPermission),
sectionService.createWithDefaultData({ ref, hash: undefined }, readPermission),
]);

const query = { hashes: {} };
query.hashes[sections[0]._id] = 'firsthash';
query.hashes[sections[1]._id] = 'wronghash';
query.hashes = JSON.stringify(query.hashes);
const result = await sectionService.sendRequestToThisService('find', {
userId, lessonId, query,
});
expect(result.status).to.equal(200);
expect(result.data.data).to.have.lengthOf(3);
expect(result.data.data).all.have.property(userScopePermissionKey);
expect(result.data.data[0]).to.not.have.property('state');
expect(result.data.data[1]).to.have.property('state');
expect(result.data.data[2]).to.have.property('state');
});


it('patch with write permissions should work', async () => {
const { _id: lessonId } = await lessonService.createWithDefaultData({ courseId }, writePermission);
Expand Down
5 changes: 4 additions & 1 deletion src/services/sections/utils/index.js
Original file line number Diff line number Diff line change
@@ -1 +1,4 @@
exports.diffToMongo = require('./diffToMongo');
const { filterManyByHashes, filterStateByHash } = require('./versionHashes');
const diffToMongo = require('./diffToMongo');

module.exports = { diffToMongo, filterManyByHashes, filterStateByHash };
40 changes: 40 additions & 0 deletions src/services/sections/utils/versionHashes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
const { BadRequest } = require('@feathersjs/errors');

/**
* if the hash matches the hash of the section, this function filters any data that is not required by the client
* (since he already has the most recent version of the section)
* @param {Object} section a section
* @param {String} hash string, representing a hash of a version of the section
*/
const filterStateByHash = (section, hash) => {
const clientAlreadyHasNewestState = hash && hash === section.hash;
if (clientAlreadyHasNewestState) {
return {
_id: section._id,
permissions: section.permissions,
title: section.title,
hash: section.hash,
};
}
return section;
};

/**
* applies filterStateByHash on an array of sections, using a map of hashes
* @param {Array} sections an array of sections.
* @param {stringified JSON} hashes Object with sectionIds as keys, and hashes as values.
*/
const filterManyByHashes = (sections, hashes) => {
if (hashes) {
try {
// eslint-disable-next-line no-param-reassign
hashes = JSON.parse(hashes);
} catch (err) {
throw new BadRequest('hashes should be valid stringified JSON');
}
return sections.map((section) => filterStateByHash(section, hashes[section._id.toString()]));
}
return sections;
};

module.exports = { filterManyByHashes, filterStateByHash };
2 changes: 1 addition & 1 deletion src/testHelpers/testHelper.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable arrow-body-style */
/* eslint-disable arrow-body-style, max-classes-per-file */
const mongoose = require('mongoose');
const axios = require('axios');
const queryString = require('query-string');
Expand Down