Replace query string with url params

README.md

@@ -16,7 +16,7 @@ Run `npm test`.
     - Success: JSON representation of the saved [mail](#mail-model-specification)
     - Fail: 500 Error
 - Show mails list
-  - GET /api/mails?recipient=*recipient*
+  - GET /api/recipient/*:recipient*/mails
   - Response
     - Success: List of [mail](#mail-model-specification)s represented as JSON
       ```javascript
@@ -25,24 +25,22 @@ Run `npm test`.
       - There is `isSecret` boolean field
       - Respond empty array when there is no mail which is matched query
     - Fail
-      - Respond 400 Bad Request if the 'recipient' GET parameter is missing
+      - Respond 404 Not Found if the 'recipient' params is missing
 - Show a non-secret mail
-  - GET /api/mails/*mailId*?recipient=*recipient*
+  - GET /api/recipient/*:recipient*/mails/*:id*
   - Response
     - Success: A [mail](#mail-model-specification) represented as JSON
     - Fail
-      - Respond 400 Bad Request if any GET parameter is missing
-      - Respond 404 Not Found if the requested mail is not present
+      - Respond 404 Not Found if the 'recipient' params is missing or the requested mail is not present
 - Show a secret mail
-  - GET /api/mails/*mailId*?recipient=*recipient*
+  - GET /api/recipient/*:recipient*/mails/*:id*
   - Header required
     - Sh8-Secret-Code: the secretCode
   - Response
     - Success: A [mail](#mail-model-specification) represented as JSON
     - Fail
-      - Respond 400 Bad Request if any GET parameter is missing
+      - Respond 404 Not Found if the 'recipient' params is missing or the requested mail is not present
       - Respond 403 Forbidden if the secretCode is invalid
-      - Respond 404 Not Found if the requested mail is not present
 
 ## Mail Model Specification
 

app.js

@@ -38,14 +38,7 @@ app.use(bodyParser.json())
 app.use(bodyParser.urlencoded({ extended: false }))
 app.use(cookieParser())
 
-app.use('/api/mails', mails)
-
-// catch 404 and forward to error handler
-app.use((req, res, next) => {
-  const err = new Error('Not Found')
-  err.status = 404
-  next(err)
-})
+app.use('/api/', mails)
 
 // error handler
 app.use((err, req, res, next) => {
@@ -54,10 +47,17 @@ app.use((err, req, res, next) => {
   res.locals.error = req.app.get('env') === 'development' ? err : {}
 
   winston.error(err)
+
   res.status(err.status || 500)
   res.send({
     message: err.message,
   })
 })
 
+// catch 404
+app.use((req, res, next) => {
+  winston.info(`Not Found: ${req.originalUrl}`)
+  res.sendStatus(404)
+})
+
 module.exports = app

routes/mails.js

@@ -4,7 +4,7 @@ const Mail = require('../models/mail')
 const router = express.Router()
 
 /* POST create a mail */
-router.post('/create', (req, res, next) => {
+router.post('/mails/create', (req, res, next) => {
   const mail = new Mail()
   mail.subject = req.body.subject
   mail.recipient = req.body.recipient
@@ -25,8 +25,8 @@ router.post('/create', (req, res, next) => {
 })
 
 /* GET show mails */
-router.get('/', (req, res, next) => {
-  const recipient = req.query.recipient
+router.get('/recipient/:recipient/mails', (req, res, next) => {
+  const recipient = req.params.recipient
 
   if (!recipient) {
     res.sendStatus(400)
@@ -52,8 +52,8 @@ router.get('/', (req, res, next) => {
 })
 
 /* GET show a mail */
-router.get('/:mailId', (req, res, next) => {
-  const recipient = req.query.recipient
+router.get('/recipient/:recipient/mails/:mailId', (req, res, next) => {
+  const recipient = req.params.recipient
   const mailId = req.params.mailId
 
   if (!recipient || !mailId) {
@@ -67,7 +67,7 @@ router.get('/:mailId', (req, res, next) => {
   }).exec().then((mail) => {
     if (!mail) {
       res.sendStatus(404)
-      return
+      next()
     }
     if (mail.isSecret && mail.secretCode !== req.header('Sh8-Secret-Code')) {
       res.sendStatus(403)

test/integration/mail-api.js

@@ -62,7 +62,7 @@ describe('POST /api/mails/create', function() {
   })
 })
 
-describe('GET /api/mails', function() {
+describe('GET /api/recipient/:recipient/mails', function() {
   const fixture = {
     mails: [],
     recipient: 'getogrand',
@@ -78,7 +78,7 @@ describe('GET /api/mails', function() {
   })
 
   it('should respond mails list successfully', function () {
-    return request(app).get(`/api/mails?recipient=${fixture.recipient}`).expect(200).then((res) => {
+    return request(app).get(`/api/recipient/${fixture.recipient}/mails`).expect(200).then((res) => {
       const mails = res.body
       mails.length.should.equal(fixture.mails.length)
       mails.forEach((actual, i) => {
@@ -103,19 +103,19 @@ describe('GET /api/mails', function() {
     })
   })
 
-  it('should respond empty array when there is no mail which is matched query', function() {
+  it('should respond empty array when there is no mail which is matched params', function() {
     const recipient = 'not_exist_recipient'
-    return request(app).get(`/api/mails?recipient=${recipient}`).expect(200).then((res) => {
+    return request(app).get(`/api/recipient/${recipient}/mails`).expect(200).then((res) => {
       res.body.should.deepEqual([])
     })
   })
 
-  it('should respond 400 Bad Request if the \'recipient\' GET parameter is missing', function() {
-    return request(app).get('/api/mails').expect(400)
+  it('should respond 404 Not Found if the \'recipient\' params is missing', function() {
+    return request(app).get('/api/mails').expect(404)
   })
 })
 
-describe('GET /api/mails/:mailId', function() {
+describe('GET /api/recipient/:recipient/mails/:mailId', function() {
   describe('CASE: non-secret mail', function() {
     const fixture = {
       mails: [],
@@ -129,21 +129,21 @@ describe('GET /api/mails/:mailId', function() {
 
     it('should respond a non-secret mail successfully', function() {
       const expected = fixture.mails[1]
-      return request(app).get(`/api/mails/${expected.id}?recipient=${expected.recipient}`).expect(200).then((res) => {
+      return request(app).get(`/api/recipient/${expected.recipient}/mails/${expected.id}`).expect(200).then((res) => {
         const actual = res.body
         assertMailResponse(actual, expected)
       })
     })
 
-    it('should respond 400 Bad Request if the \'recipient\' GET parameter is missing', function() {
+    it('should respond 404 Not Found if the \'recipient\' params is missing', function() {
       const expected = fixture.mails[1]
-      return request(app).get(`/api/mails/${expected.id}`).expect(400)
+      return request(app).get(`/api/mails/${expected.id}`).expect(404)
     })
 
     it('should respond 404 Not Found if the requested mail is not present', function() {
       const expected = fixture.mails[1]
       const id = 'not_exist_id'
-      return request(app).get(`/api/mails/${id}?recipient=${expected.recipient}`).expect(404)
+      return request(app).get(`/api/recipient/${expected.recipient}/mails/${id}`).expect(404)
     })
   })
 
@@ -161,15 +161,15 @@ describe('GET /api/mails/:mailId', function() {
 
     it('should respond a secret mail successfully', function() {
       const expected = fixture.mails[1]
-      return request(app).get(`/api/mails/${expected.id}?recipient=${expected.recipient}`).set('Sh8-Secret-Code', expected.secretCode).expect(200).then((res) => {
+      return request(app).get(`/api/recipient/${expected.recipient}/mails/${expected.id}`).set('Sh8-Secret-Code', expected.secretCode).expect(200).then((res) => {
         const actual = res.body
         assertMailResponse(actual, expected)
       })
     })
 
     it('should respond 403 Forbidden if the secretCode is invalid', function () {
       const expected = fixture.mails[1]
-      return request(app).get(`/api/mails/${expected.id}?recipient=${expected.recipient}`).set('Sh8-Secret-Code', 'invalid_password_1234').expect(403)
+      return request(app).get(`/api/recipient/${expected.recipient}/mails/${expected.id}`).set('Sh8-Secret-Code', 'invalid_password_1234').expect(403)
     })
   })
 })