Skip to content

Taiga Task 426 : Deployment of FLOSS document management system #45

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
almereyda wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Taiga Task 426 : Deployment of FLOSS document management system #45

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
c4e3467
T426 : add 'media' type
almereyda Aug 4, 2017
2c1e7ee
More natural language 500 error messages.
almereyda Aug 8, 2017
d64f266
modify docker build for local development environment
almereyda Aug 8, 2017
2a25bdc
rename things integration test to things-<type> (place)
almereyda Aug 8, 2017
12ae79a
update lint environment to recognize Mocha js and remove it & describ…
almereyda Aug 8, 2017
709f738
add package-lock.json
almereyda Aug 8, 2017
05ee776
add first attempt of integration test for media endpoint and supply f…
almereyda Aug 8, 2017
437d426
version model : remove 42
almereyda Sep 17, 2017
c16fb9b
git hook : allow TDD and only lint, not test upon commit
almereyda Sep 17, 2017
e471f7f
test : Media skeleton from Place
almereyda Sep 17, 2017
688ea8e
update : package + lock
almereyda Sep 19, 2017
bb5f8ad
update : fixtures
almereyda Sep 19, 2017
95069e3
IPFS configuration
Sep 28, 2017
222c81f
Merge remote-tracking branch 'origin/replaceAmericano' into T426
Sep 28, 2017
0069acd
IPFS configuration using multiaddr
Sep 28, 2017
7cb1152
multipart uploading skeleton
Sep 28, 2017
4130175
additional packages for multipart and IPFS
Sep 28, 2017
9fdd561
move upload logic to put routes
Sep 30, 2017
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
18 changes: 15 additions & 3 deletions server/controllers/things/build_controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,20 @@ module.exports = function (typeName) {
}
},
post: function (req, res) {
lib.create(req.body)
.then(res.json.bind(res))
.catch(error_.Handler(res))
if (req.files) {
lib.upload(req.body, req.files.mediaUpload) // code smell: hard coded input id, configurable, or per type?
Copy link
Copy Markdown

@acorbi acorbi Sep 29, 2017

Choose a reason for hiding this comment

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

@almereyda

Do I understand correctly that:

req.body would be the metadata of the media file and req.files.mediaUpload the binary contents of the actual asset? Are they both POSTed on the same API method call?

On https://hack.allmende.io/transformaps-20170926-development?view#api-calls-involved-on-uploading-a-media-file-and-associating-it-with-a-poi (and as currently implemented) I am envisioning to do this in 2 calls...

If my statement is correct, our implementations are currently dissaligned. Please confirm or elaborate so we can align both implementations.

Copy link
Copy Markdown
Member Author

@almereyda almereyda Oct 1, 2017

Choose a reason for hiding this comment

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

From the sequence diagrams I learn you expect to be POSTing a BLOB to a separate /blob endpoint associated to any media/:id route. From this I like the idea to separate the creation of a media metadata container (thing) from its filling. Yet I suggest an upload can be triggered in presence of a multipart stream in any PUT action and thus more tightly couple this to a single medium's endpoint.

There may be the need for additional handling of the side effects, i.e. on other things such as place, where one might want to avoid allowing for binary uploads.

Copy link
Copy Markdown

@acorbi acorbi Oct 8, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@almereyda I have been studying your proposal of coupling the upload of the binary contents of the asset together with its metadata in a POST (creation) or PUT (update) method on /media/ and came up with following 'dilemma':

As described on https://hack.allmende.io/transformaps-20170926-development?view#api-calls-involved-on-updating-a-poi-by-uploading-a-new-asset and according to the current implementation, updating a media file does not involve a PUT call to the media endpoint but a POST call to media/{mediaId}/version with the goal of storing a new version object on the DB and not updating the media file's metadata.

I see 2 options at this point to achieve an implementation which would make sense to me:

  1. To leave the mutipart upload of the asset decoupled from the creation (POST) or update of a certain media file, as it is currently the case.

  2. To delegate the responsibility of the creation of new versions of a media file (journaling) to the data service. This means that the editor would not make any POST calls to media/{mediaId}/versions as it is currently the case ( see https://github.com/TransforMap/transformap-editor/pull/42/files#diff-e2815ec3044d7d3fc5c27a5a3ed1b358R115). The editor would just make a PUT call to media/{mediaId} and the data service would then store a new version of it.

.then(res.json.bind(res))
.catch(error_.Handler(res))
} else {
lib.create(req.body)
.then(res.json.bind(res))
.catch(error_.Handler(res))
}


},
put: function (req, res) {

const id = req.params.id

if (!_.isUuid(id)) {
Expand All @@ -40,8 +49,10 @@ module.exports = function (typeName) {
lib.update(req.body, id)
.then(res.json.bind(res))
.catch(error_.Handler(res))

},
delete: function (req, res) {

const id = req.params.id

if (!_.isUuid(id)) {
Expand All @@ -52,6 +63,7 @@ module.exports = function (typeName) {
lib.delete(id)
.then(res.json.bind(res))
.catch(error_.Handler(res))

}
}
}
19 changes: 19 additions & 0 deletions server/controllers/things/build_lib.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const db = __.require('lib', 'db/db')('things', 'journals')
const Journal = require('./models/commons/journal')
const Version = require('./models/commons/version')
const versions_ = require('./lib/versions')
const upload = require('./lib/upload.js')
const promises_ = __.require('lib', 'promises')
const error_ = __.require('lib', 'error')

Expand Down Expand Up @@ -127,6 +128,24 @@ module.exports = function (typeName, model) {
})
})
.then(_.Log('return value of insert'))
},
upload: function (data, file) {

try {
model.validateData(data)
} catch (err) {
return promises_.reject(err)
}

data.hash = upload(file)

return db.post(Journal.create(typeName))
.then(_.Log('journal post res'))
.then(_.property('id'))
.then(_.partial(updateJournal, data))
.then(_.Log('thing created'))
.catch(_.ErrorRethrow('thing creation err'))

}
}
}
Expand Down
15 changes: 15 additions & 0 deletions server/controllers/things/lib/upload.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
const CONFIG = require('config')
const __ = CONFIG.universalPath
const _ = __.require('lib', 'utils')
const ipfsAPI = require('ipfs-api')
const db = __.require('lib', 'db/db')('things', 'versions')
const Version = require('../models/commons/version')

var ipfs = ipfsAPI(CONFIG.blobStore.multiaddr)

module.exports = function (file) {



return
}
1 change: 1 addition & 0 deletions server/middlewares.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ module.exports = {
require('body-parser').json(),
require('method-override')(),
require('errorhandler')({ dumpExceptions: true, showStack: true }),
require('express-fileupload')(),
cors
],
development: [
Expand Down