Skip to content

LDP constructor cleanup #705

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

Merged
dmitrizagidulin merged 8 commits into from
Jun 14, 2018
Merged

LDP constructor cleanup #705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ca0d3c7
Remove 'extend' dependency
dmitrizagidulin Jun 11, 2018
ce74811
Remove unused 'skin' param
dmitrizagidulin Jun 11, 2018
5aa508a
Extract initErrorPages() out of LDP constructor
dmitrizagidulin Jun 11, 2018
491c23c
Move printDebugInfo() to server-config.js
dmitrizagidulin Jun 11, 2018
859cb0e
Explicitly assign LDP options in constructor (instead of Object.assign)
dmitrizagidulin Jun 11, 2018
cba0bbc
Remove 'auth' as ldp param - not needed
dmitrizagidulin Jun 11, 2018
af604a6
Update oidc-auth-manager dep to 0.15.3
dmitrizagidulin Jun 11, 2018
0d61d90
Update oidc-auth-manager to 0.16.0
dmitrizagidulin Jun 14, 2018
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
3 changes: 1 addition & 2 deletions bin/lib/start.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

const options = require('./options')
const fs = require('fs')
const extend = require('extend')
const colors = require('colors/safe')

module.exports = function (program, server) {
Expand All @@ -23,7 +22,7 @@ module.exports = function (program, server) {
start.option('-v, --verbose', 'Print the logs to console')

start.action((opts) => {
let argv = extend({}, opts, { version: program.version() })
let argv = Object.assign({}, opts, { version: program.version() })
let configFile = argv['configFile'] || './config.json'

fs.readFile(configFile, (err, file) => {
Expand Down
4 changes: 3 additions & 1 deletion config/defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ module.exports = {
'port': 8443,
'serverUri': 'https://localhost:8443',
'webid': true,
'dataBrowserPath': 'default'
'dataBrowserPath': 'default',
'suffixAcl': '.acl',
'suffixMeta': '.meta'

// For use in Enterprises to configure a HTTP proxy for all outbound HTTP requests from the SOLID server (we use
// https://www.npmjs.com/package/global-tunnel-ng).
Expand Down
3 changes: 2 additions & 1 deletion lib/create-app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,9 @@ function createApp (argv = {}) {
const configPath = config.initConfigDir(argv)
argv.templates = config.initTemplateDirs(configPath)

config.printDebugInfo(argv)

const ldp = new LDP(argv)
ldp.printDebugInfo()

const app = express()

Expand Down
4 changes: 2 additions & 2 deletions lib/create-server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ function createServer (argv, app) {

let server
const needsTLS = argv.sslKey || argv.sslCert ||
(ldp.webid || ldp.multiuser) && !argv.certificateHeader
(argv.webid || argv.multiuser) && !argv.certificateHeader
if (!needsTLS) {
server = http.createServer(app)
} else {
Expand Down Expand Up @@ -73,7 +73,7 @@ function createServer (argv, app) {
cert: cert
}, argv)

if (ldp.webid && ldp.auth === 'tls') {
if (argv.webid && argv.auth === 'tls') {
credentials.requestCert = true
}

Expand Down
74 changes: 29 additions & 45 deletions lib/ldp.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ const utils = require('./utils')
const error = require('./http-error')
const stringToStream = require('./utils').stringToStream
const serialize = require('./utils').serialize
const extend = require('extend')
const rimraf = require('rimraf')
const ldpContainer = require('./ldp-container')
const parse = require('./utils').parse
const defaults = require('../config/defaults')

const DEFAULT_CONTENT_TYPE = 'text/turtle'

Expand Down Expand Up @@ -44,67 +44,51 @@ const RDF_MIME_TYPES = [
* @param [options.suppressDataBrowser]
* @param [options.dataBrowserPath]
* @param [options.webid]
* @param [options.auth]
* @param [options.multiuser]
* @param [options.proxy]
* @param [options.live]
* @param [options.store]
* @param [options.corsProxy]
*/
class LDP {
constructor (argv = {}) {
extend(this, argv)
constructor (options = {}) {
options = Object.assign({}, defaults, options)
this.serverUri = options.serverUri
this.dbPath = options.dbPath
this.configPath = options.configPath
this.suffixAcl = options.suffixAcl
this.suffixMeta = options.suffixMeta
this.multiuser = options.multiuser
this.webid = options.webid
this.strictOrigin = options.strictOrigin
this.live = options.live
this.errorPages = options.errorPages
this.noErrorPages = options.noErrorPages
this.errorHandler = options.errorHandler
this.dataBrowserPath = options.dataBrowserPath
this.suppressDataBrowser = options.suppressDataBrowser

// Setting root
if (!this.root) {
this.root = process.cwd()
}
if (!this.root.endsWith('/')) {
this.root += '/'
}
this.turtleExtensions = [ '.ttl', this.suffixAcl, this.suffixMeta ]

// Suffixes
if (!this.suffixAcl) {
this.suffixAcl = '.acl'
}
if (!this.suffixMeta) {
this.suffixMeta = '.meta'
this.root = options.root || process.cwd()
if (!this.root.endsWith('/')) { this.root += '/' }

if (this.corsProxy && !this.corsProxy.startsWith('/')) {
this.corsProxy = '/' + this.corsProxy

@RubenVerborgh RubenVerborgh Jun 11, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So the above is fine, but when I code, I tend to have the rule that the length of the code should represent the complexity of the action. My suggestion would be:

this.root.replace(/\/*$/, '/');
this.corsProxy.replace(/$\/*/, '/');

@dmitrizagidulin dmitrizagidulin Jun 11, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Does the regex variant take care of the if (this.corsProxy && guard?

(I must admit, though I like the conciseness of regex, since i'm less familiar with it, I find the more wordy if/then more readable :) )

@RubenVerborgh RubenVerborgh Jun 11, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah no, doesn't have that guard. Leave it as-is then, readability is king.

}
this.turtleExtensions = [ '.ttl', this.suffixAcl, this.suffixMeta ]

// Error pages folder
this.initErrorPages(options)
}

initErrorPages (options) {
this.errorPages = null
if (!this.noErrorPages) {
this.errorPages = argv.errorPages
this.errorPages = options.errorPages
if (!this.errorPages) {
// TODO: For now disable error pages if errorPages parameter is not explicitly passed
this.noErrorPages = true
} else if (!this.errorPages.endsWith('/')) {
this.errorPages += '/'
}
}

if (this.skin !== false) {
this.skin = true
}

if (this.corsProxy && this.corsProxy[ 0 ] !== '/') {
this.corsProxy = '/' + this.corsProxy
}
}

printDebugInfo () {
debug.settings('Server URI: ' + this.serverUri)
debug.settings('Auth method: ' + this.auth)
debug.settings('Db path: ' + this.dbPath)
debug.settings('Config path: ' + this.configPath)
debug.settings('Suffix Acl: ' + this.suffixAcl)
debug.settings('Suffix Meta: ' + this.suffixMeta)
debug.settings('Filesystem Root: ' + this.root)
debug.settings('Allow WebID authentication: ' + !!this.webid)
debug.settings('Live-updates: ' + !!this.live)
debug.settings('Multi-user: ' + !!this.multiuser)
debug.settings('Suppress default data browser app: ' + this.suppressDataBrowser)
debug.settings('Default data browser app file path: ' + this.dataBrowserPath)
}

stat (file, callback) {
Expand Down
19 changes: 18 additions & 1 deletion lib/server-config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,22 @@

const fs = require('fs-extra')
const path = require('path')
const debug = require('./debug')

function printDebugInfo (options) {
debug.settings('Server URI: ' + options.serverUri)
debug.settings('Auth method: ' + options.auth)
debug.settings('Db path: ' + options.dbPath)
debug.settings('Config path: ' + options.configPath)
debug.settings('Suffix Acl: ' + options.suffixAcl)
debug.settings('Suffix Meta: ' + options.suffixMeta)
debug.settings('Filesystem Root: ' + options.root)
debug.settings('Allow WebID authentication: ' + !!options.webid)
debug.settings('Live-updates: ' + !!options.live)
debug.settings('Multi-user: ' + !!options.multiuser)
debug.settings('Suppress default data browser app: ' + options.suppressDataBrowser)
debug.settings('Default data browser app file path: ' + options.dataBrowserPath)
}

/**
* Ensures that a directory has been copied / initialized. Used to ensure that
Expand Down Expand Up @@ -132,5 +148,6 @@ module.exports = {
ensureWelcomePage,
initConfigDir,
initDefaultViews,
initTemplateDirs
initTemplateDirs,
printDebugInfo
}
Loading