Clean code

Signed-off-by: nick.chen <nick.chen.sudo@gmail.com>

lib/models/archivedNoteAlias.js

@@ -1,3 +1,5 @@
+'use strict'
+// external modules
 const Sequelize = require('sequelize')
 
 module.exports = function (sequelize, DataTypes) {

lib/note/index.js

@@ -8,55 +8,13 @@ const { newCheckViewPermission, errorForbidden, responseCodiMD, errorNotFound, e
 const { updateHistory, historyDelete } = require('../history')
 const { actionPublish, actionSlide, actionInfo, actionDownload, actionPDF, actionGist, actionRevision, actionPandoc } = require('./noteActions')
 const realtime = require('../realtime/realtime')
-const serv = require('./service')
+const service = require('./service')
 
-async function getNoteById (noteId, { includeUser } = { includeUser: false }) {
+exports.showNote = async (req, res) => {
-  const id = await Note.parseNoteIdAsync(noteId)
-
-  const includes = []
-
-  if (includeUser) {
-    includes.push({
-      model: User,
-      as: 'owner'
-    }, {
-      model: User,
-      as: 'lastchangeuser'
-    })
-  }
-
-  const note = await Note.findOne({
-    where: {
-      id: id
-    },
-    include: includes
-  })
-  return note
-}
-
-async function createNote (userId, noteAlias) {
-  if (!config.allowAnonymous && !userId) {
-    throw new Error('can not create note')
-  }
-
-  const note = await Note.create({
-    ownerId: userId,
-    alias: noteAlias
-  })
-
-  if (userId) {
-    updateHistory(userId, note)
-  }
-
-  return note
-}
-
-// controller
-async function showNote (req, res) {
   const noteId = req.params.noteId
   const userId = req.user ? req.user.id : null
 
-  let note = await getNoteById(noteId)
+  let note = await service.getNote(noteId)
 
   if (!note) {
     // if allow free url enable, auto create note
@@ -65,7 +23,7 @@ async function showNote (req, res) {
     } else if (!config.allowAnonymous && !userId) {
       return errorForbidden(req, res)
     }
-    note = await createNote(userId, noteId)
+    note = await service.createNote(userId, noteId)
   }
 
   if (!newCheckViewPermission(note, req.isAuthenticated(), userId)) {
@@ -80,20 +38,10 @@ async function showNote (req, res) {
   return responseCodiMD(res, note)
 }
 
-function canViewNote (note, isLogin, userId) {
+exports.showPublishNote = async (req, res) => {
-  if (note.permission === 'private') {
-    return note.ownerId === userId
-  }
-  if (note.permission === 'limited' || note.permission === 'protected') {
-    return isLogin
-  }
-  return true
-}
-
-async function showPublishNote (req, res) {
   const shortid = req.params.shortid
 
-  const note = await getNoteById(shortid, {
+  const note = await service.getNote(shortid, {
     includeUser: true
   })
 
@@ -101,12 +49,12 @@ async function showPublishNote (req, res) {
     return errorNotFound(req, res)
   }
 
-  if (!canViewNote(note, req.isAuthenticated(), req.user ? req.user.id : null)) {
+  if (!service.canViewNote(note, req.isAuthenticated(), req.user ? req.user.id : null)) {
     return errorForbidden(req, res)
   }
 
   if ((note.alias && shortid !== note.alias) || (!note.alias && shortid !== note.shortid)) {
-    return res.redirect(config.serverURL + '/s/' + (note.alias || note.shortid))
+    return res.redirect(config.serviceerURL + '/s/' + (note.alias || note.shortid))
   }
 
   await note.increment('viewcount')
@@ -144,16 +92,16 @@ async function showPublishNote (req, res) {
   res.render('pretty.ejs', data)
 }
 
-async function noteActions (req, res) {
+exports.noteActions = async (req, res) => {
   const noteId = req.params.noteId
 
-  const note = await getNoteById(noteId)
+  const note = await service.getNote(noteId)
 
   if (!note) {
     return errorNotFound(req, res)
   }
 
-  if (!canViewNote(note, req.isAuthenticated(), req.user ? req.user.id : null)) {
+  if (!service.canViewNote(note, req.isAuthenticated(), req.user ? req.user.id : null)) {
     return errorForbidden(req, res)
   }
 
@@ -188,41 +136,13 @@ async function noteActions (req, res) {
       actionPandoc(req, res, note)
       break
     default:
-      return res.redirect(config.serverURL + '/' + noteId)
+      return res.redirect(config.serviceerURL + '/' + noteId)
   }
 }
 
-async function getMyNoteList (userId, callback) {
+exports.listMyNotes = (req, res) => {
-  const myNotes = await Note.findAll({
-    where: {
-      ownerId: userId
-    }
-  })
-  if (!myNotes) {
-    return callback(null, null)
-  }
-  try {
-    const myNoteList = myNotes.map(note => ({
-      id: Note.encodeNoteId(note.id),
-      text: note.title,
-      tags: Note.parseNoteInfo(note.content).tags,
-      createdAt: note.createdAt,
-      lastchangeAt: note.lastchangeAt,
-      shortId: note.shortid
-    }))
-    if (config.debug) {
-      logger.info('Parse myNoteList success: ' + userId)
-    }
-    return callback(null, myNoteList)
-  } catch (err) {
-    logger.error('Parse myNoteList failed')
-    return callback(err, null)
-  }
-}
-
-function listMyNotes (req, res) {
   if (req.isAuthenticated()) {
-    getMyNoteList(req.user.id, (err, myNoteList) => {
+    service.getMyNoteList(req.user.id, (err, myNoteList) => {
       if (err) return errorInternalError(req, res)
       if (!myNoteList) return errorNotFound(req, res)
       res.send({
@@ -234,7 +154,7 @@ function listMyNotes (req, res) {
   }
 }
 
-const deleteNote = async (req, res) => {
+exports.deleteNote = async (req, res) => {
   if (req.isAuthenticated()) {
     const noteId = await Note.parseNoteIdAsync(req.params.noteId)
     try {
@@ -268,7 +188,7 @@ const deleteNote = async (req, res) => {
   }
 }
 
-const updateNote = async (req, res) => {
+exports.updateNote = async (req, res) => {
   if (req.isAuthenticated() || config.allowAnonymousEdits) {
     const noteId = await Note.parseNoteIdAsync(req.params.noteId)
     try {
@@ -333,65 +253,42 @@ const updateNote = async (req, res) => {
   }
 }
 
-const checkAliasValid = async (req, res) => {
+exports.updateNoteAlias = async (req, res) => {
-  const originAliasOrNoteId = req.params.originAliasOrNoteId
-  const alias = req.query.alias
-  const isValid = await serv.asyncCheckAliasValid(originAliasOrNoteId, alias)
-    .catch((err) => {
-      logger.error(err.message)
-      return res.status(500).send('Internal Error.')
-    })
-
-  res.send({
-    status: 'ok',
-    isValid
-  })
-}
-
-const updateNoteAlias = async (req, res) => {
   const originAliasOrNoteId = req.params.originAliasOrNoteId
   const alias = req.body.alias || ''
   const userId = req.user ? req.user.id : null
-  const note = await serv.asyncGetNote(originAliasOrNoteId)
+  const note = await service.getNote(originAliasOrNoteId)
     .catch((err) => {
       logger.error('get note failed:' + err.message)
       return false
     })
 
+  if (!note) {
+    logger.error('update note alias failed: note not found.')
+  }
+
   if (note.ownerId !== userId) {
-    return res.status(403).send('Forbidden.')
+    return res.status(403).json({ status: 'error', message: 'Forbidden' })
   }
 
-  const isValid = await serv.asyncCheckAliasValid(originAliasOrNoteId, alias)
+  const result = await service.updateAlias(originAliasOrNoteId, alias)
-    .catch((err) => {
-      logger.error(err.message)
-      return res.status(500).send('Internal Error.')
-    })
-
-  if (!isValid) {
-    return res.status(400).send('Bad Request.')
-  }
-
-  const isSuccess = await serv.asyncUpdateAlias(originAliasOrNoteId, alias)
     .catch((err) => {
       logger.error('update note alias failed:' + err.message)
       return false
     })
 
+  if (!result) {
+    return res.status(500).json({ status: 'error', message: 'Internal Error' })
+  }
+
+  const { isSuccess, errorMessage } = result
+
   if (!isSuccess) {
-    return res.status(500).send('Internal Error.')
+    res.status(400).json({ status: 'error', message: errorMessage })
+    return
   }
 
   res.send({
     status: 'ok'
   })
 }
-
-exports.showNote = showNote
-exports.showPublishNote = showPublishNote
-exports.noteActions = noteActions
-exports.listMyNotes = listMyNotes
-exports.deleteNote = deleteNote
-exports.updateNote = updateNote
-exports.checkAliasValid = checkAliasValid
-exports.updateNoteAlias = updateNoteAlias

lib/note/service.js

@@ -1,68 +1,126 @@
 
-const { Note, ArchivedNoteAlias, sequelize } = require('../models')
+'use strict'
+const { Note, User, ArchivedNoteAlias, sequelize } = require('../models')
+const config = require('../config')
+const logger = require('../logger')
 const realtime = require('../realtime/realtime')
 
-const forbiddenAlias = ['', 'new', 'me', 'history', '403', '404', '500', 'config']
+const EXIST_WEB_ROUTES = ['', 'new', 'me', 'history', '403', '404', '500', 'config']
-const sanitize = (alias) => {
+const FORBIDDEN_ALIASES = [...EXIST_WEB_ROUTES, ...config.forbiddenNoteIDs]
-  return alias.replace(/( |\/)/, '')
+
-}
+exports.getNote = async (originAliasOrNoteId, { includeUser } = { includeUser: false }) => {
+  const id = await Note.parseNoteIdAsync(originAliasOrNoteId)
+
+  const includes = []
+
+  if (includeUser) {
+    includes.push({
+      model: User,
+      as: 'owner'
+    }, {
+      model: User,
+      as: 'lastchangeuser'
+    })
+  }
 
-const asyncGetNote = async (originAliasOrNoteId) => {
-  const noteId = await Note.parseNoteIdAsync(originAliasOrNoteId)
   const note = await Note.findOne({
     where: {
-      id: noteId
+      id: id
-    }
+    },
+    include: includes
   })
-  if (!note) {
-    throw Error('Can\'t find the note.')
-  }
   return note
 }
 
-const asyncGetNoteIdForAliasConflict = async (alias) => {
+exports.canViewNote = (note, isLogin, userId) => {
-  const sanitizedAlias = sanitize(alias)
+  if (note.permission === 'private') {
-  const p1 = Note.findOne({
+    return note.ownerId === userId
-    where: {
-      alias: sanitizedAlias
-    }
-  })
-  const p2 = ArchivedNoteAlias.findOne({
-    where: {
-      alias: sanitizedAlias
-    }
-  })
-  const [conflictNote, conflictAarchivedAlias] = await Promise.all([p1, p2])
-
-  if (conflictNote) {
-    return conflictNote.id
   }
-  if (conflictAarchivedAlias) {
+  if (note.permission === 'limited' || note.permission === 'protected') {
-    return conflictAarchivedAlias.noteId
+    return isLogin
   }
-  return null
+  return true
 }
 
-const asyncCheckAliasValid = async (originAliasOrNoteId, alias) => {
+exports.getMyNoteList = async (userId, callback) => {
-  const sanitizedAlias = sanitize(alias)
+  const myNotes = await Note.findAll({
-  if (forbiddenAlias.indexOf(sanitizedAlias) > -1) {
+    where: {
-    return false
+      ownerId: userId
+    }
+  })
+  if (!myNotes) {
+    return callback(null, null)
+  }
+  try {
+    const myNoteList = myNotes.map(note => ({
+      id: Note.encodeNoteId(note.id),
+      text: note.title,
+      tags: Note.parseNoteInfo(note.content).tags,
+      createdAt: note.createdAt,
+      lastchangeAt: note.lastchangeAt,
+      shortId: note.shortid
+    }))
+    if (config.debug) {
+      logger.info('Parse myNoteList success: ' + userId)
+    }
+    return callback(null, myNoteList)
+  } catch (err) {
+    logger.error('Parse myNoteList failed')
+    return callback(err, null)
   }
-
-  const conflictNoteId = await asyncGetNoteIdForAliasConflict(alias)
-    .catch((err) => { throw err })
-
-  const note = await asyncGetNote(originAliasOrNoteId)
-    .catch((err) => { throw err })
-
-  return !conflictNoteId || conflictNoteId === note.id
 }
 
-const asyncUpdateAlias = async (originAliasOrNoteId, alias) => {
+const sanitizeAlias = (alias) => {
-  const sanitizedAlias = sanitize(alias)
+  return alias.replace(/( |\/)/g, '')
-  const note = await asyncGetNote(originAliasOrNoteId)
+}
+
+const checkAliasValid = async (originAliasOrNoteId, alias) => {
+  const sanitizedAlias = sanitizeAlias(alias)
+  if (FORBIDDEN_ALIASES.includes(sanitizedAlias)) {
+    return {
+      isValid: false,
+      errorMessage: 'The url is exist.'
+    }
+  }
+
+  if (!/^[0-9a-z-_]+$/.test(alias)) {
+    return {
+      isValid: false,
+      errorMessage: 'The url must be lowercase letters, decimal digits, hyphen or underscore.'
+    }
+  }
+
+  const conflictNote = await exports.getNote(alias)
     .catch((err) => { throw err })
 
+  const note = await exports.getNote(originAliasOrNoteId)
+    .catch((err) => { throw err })
+
+  if (conflictNote && conflictNote.id !== note.id) {
+    return {
+      isValid: false,
+      errorMessage: 'The url is exist.'
+    }
+  }
+
+  return {
+    isValid: true
+  }
+}
+
+exports.updateAlias = async (originAliasOrNoteId, alias) => {
+  const sanitizedAlias = sanitizeAlias(alias)
+  const note = await exports.getNote(originAliasOrNoteId)
+    .catch((err) => { throw err })
+
+  const { isValid, errorMessage } = await checkAliasValid(originAliasOrNoteId, alias)
+  if (!isValid) {
+    return {
+      isSuccess: false,
+      errorMessage
+    }
+  }
+
   const t = await sequelize.transaction()
   if (note.alias) {
     const archivedAlias = await ArchivedNoteAlias.findOne({
@@ -71,6 +129,7 @@ const asyncUpdateAlias = async (originAliasOrNoteId, alias) => {
       }
     })
       .catch(async err => { throw err })
+
     if (!archivedAlias) {
       await ArchivedNoteAlias.create({
         noteId: note.id,
@@ -99,10 +158,7 @@ const asyncUpdateAlias = async (originAliasOrNoteId, alias) => {
       alias: updatedNote.alias
     })
 
-  return true
+  return {
+    isSuccess: true
+  }
 }
-
-exports.sanitize = sanitize
-exports.asyncGetNote = asyncGetNote
-exports.asyncCheckAliasValid = asyncCheckAliasValid
-exports.asyncUpdateAlias = asyncUpdateAlias

lib/routes.js

@@ -71,8 +71,6 @@ appRouter.get('/s/:shortid/:action', response.publishNoteActions)
 appRouter.get('/p/:shortid', response.showPublishSlide)
 // publish slide actions
 appRouter.get('/p/:shortid/:action', response.publishSlideActions)
-// check note alais valid
-appRouter.get('/api/notes/:originAliasOrNoteId/checkAliasValid', noteController.checkAliasValid)
 // update note alias
 appRouter.patch('/api/notes/:originAliasOrNoteId/alias', bodyParser.json(), noteController.updateNoteAlias)
 // get my note list

public/js/index.js

@@ -1268,21 +1268,6 @@ $('#revisionModalRevert').click(function () {
 })
 
 // custom note url modal
-const checkNoteUrlValid = (noteUrl = '') => {
-  return new Promise((resolve) => {
-    $.ajax({
-      method: 'GET',
-      url: `/api/notes/${noteid}/checkAliasValid`,
-      data: {
-        alias: noteUrl
-      },
-      success: (data) => {
-        resolve(data.isValid)
-      }
-    })
-  })
-}
-
 const updateNoteUrl = (noteUrl = '') => {
   return new Promise((resolve, reject) => {
     $.ajax({
@@ -1292,9 +1277,7 @@ const updateNoteUrl = (noteUrl = '') => {
         alias: noteUrl
       }),
       contentType: 'application/json;charset=utf-8',
-      success: (data) => {
+      success: resolve,
-        resolve(data.status === 'ok')
-      },
       error: reject
     })
   })
@@ -1309,27 +1292,34 @@ ui.modal.customNoteUrl.on('submit', function (e) {
   const hideErrorMessage = () => ui.modal.customNoteUrl.find('.alert').hide()
 
   const customUrl = ui.modal.customNoteUrl.find('[name="custom-url"]').val()
-  checkNoteUrlValid(customUrl)
+  if (!/^[0-9a-z-_]+$/.test(customUrl)) {
-    .then(isValid => {
+    showErrorMessage('The url must be lowercase letters, decimal digits, hyphen or underscore.')
-      if (!isValid) {
+    return
-        showErrorMessage('The url is exist.')
+  }
-        return
+
+  updateNoteUrl(customUrl)
+    .then(
+      ({ status }) => {
+        if (status === 'ok') {
+          hideErrorMessage()
+          ui.modal.customNoteUrl.modal('hide')
+        }
+      },
+      err => {
+        console.log(err)
+        if (err.status === 400 && err.responseJSON.message) {
+          showErrorMessage(err.responseJSON.message)
+          return
+        }
+        if (err.status === 403) {
+          showErrorMessage('Only note owner can edit custom url.')
+          return
+        }
+        showErrorMessage('Something wrong.')
       }
-      hideErrorMessage()
+    )
-      return updateNoteUrl(customUrl)
+    .catch(() => {
-    })
+      showErrorMessage('Something wrong.')
-    .then(isSuccess => {
-      if (isSuccess) {
-        hideErrorMessage()
-        ui.modal.customNoteUrl.modal('hide')
-      }
-    }, err => {
-      if (err.status === 403) {
-        showErrorMessage('Only note owner can edit custom url.')
-      }
-    })
-    .catch((err) => {
-      showErrorMessage('Something wrong: ' + err.message)
     })
 })