feat: add request.routeOptions object by debadutta98 · Pull Request #4397 · fastify/fastify

debadutta98 · 2022-11-02T19:39:02Z

Hi,

This PR Closes #4375

Work

Add request.routeOptions.bodyLimit to request (update request.js)
Add some test cases of current changes to bodyLimit.test.js
Update type declaration request.d.ts

Checklist

run npm run test and npm run benchmark
tests and/or benchmarks are included
documentation is changed or added
commit message and code follows the Developer's Certification of Origin
and the Code of conduct

mcollina

lgtm

climba03003

I do not block if it landed.
Still, I think mapping the options to property one by one is not a good idea.

Details on #4375 (comment)

docs/Reference/Request.md

lib/request.js

test/bodyLimit.test.js

docs/Reference/Request.md

debadutta98 · 2022-11-03T13:34:04Z

Hi @Eomm @climba03003 ,

I just changed request.routeBodyLimit to request.routeOptions.bodyLimit according to that doc and test cases are also
please confirm if it needs further improvement

docs/Reference/Request.md

lib/request.js

jsumners

This does not fully consider, nor implement, the ideas presented in the issue discussion.

mcollina

lgtm

…e testcases

lib/request.js

debadutta98 · 2022-11-04T05:37:52Z

Hi guys,

Just validate my solution to match your expectations

Add some new property request.routeOptions and they are only readable (if you want to include more property please let me know)
As introduced some changes to lib/request.js that way I write some test cases on request-error.test.js and modify the types of routeOptions on types/request.d.ts
Update doc on doc/References/Request.md

Thank you!

lib/request.js

docs/Reference/Request.md

Eomm

AWESOME!

Uzlopak · 2022-11-05T11:19:48Z

Can we modify the test in request-error.js to this please?
Just in case we have in future some routeOption being an Object or Array, we could overlook, that Object.freeze does not cover them properly.

test('request.routeOptions should be immutable', t => {
  t.plan(14)
  const fastify = Fastify()
  const handler = function (req, res) {
    t.equal('POST', req.routeOptions.method)
    t.equal('/', req.routeOptions.url)
    t.throws(() => { req.routeOptions = null }, new TypeError('Cannot set property routeOptions of #<Request> which has only a getter'))
    t.throws(() => { req.routeOptions.method = 'INVALID' }, new TypeError('Cannot assign to read only property \'method\' of object \'#<Object>\''))
    t.throws(() => { req.routeOptions.url = '//' }, new TypeError('Cannot assign to read only property \'url\' of object \'#<Object>\''))
    t.throws(() => { req.routeOptions.bodyLimit = 0xDEADBEEF }, new TypeError('Cannot assign to read only property \'bodyLimit\' of object \'#<Object>\''))
    t.throws(() => { req.routeOptions.attachValidation = true }, new TypeError('Cannot assign to read only property \'attachValidation\' of object \'#<Object>\''))
    t.throws(() => { req.routeOptions.logLevel = 'invalid' }, new TypeError('Cannot assign to read only property \'logLevel\' of object \'#<Object>\''))
    t.throws(() => { req.routeOptions.version = '95.0.1' }, new TypeError('Cannot assign to read only property \'version\' of object \'#<Object>\''))
    t.throws(() => { req.routeOptions.prefixTrailingSlash = true }, new TypeError('Cannot assign to read only property \'prefixTrailingSlash\' of object \'#<Object>\''))
    t.throws(() => { req.routeOptions.newAttribute = {} }, new TypeError('Cannot add property newAttribute, object is not extensible'))

    for (const key of Object.keys(req.routeOptions)) {
      if (typeof req.routeOptions[key] === 'object' && req.routeOptions[key] !== null) {
        t.fail('Object.freeze must run recursively on nested structures to ensure that routeOptions is immutable.')
      }
    }

    res.send({ })
  }
  fastify.post('/', {
    bodyLimit: 1000,
    handler
  })
  fastify.listen({ port: 0 }, function (err) {
    t.error(err)
    t.teardown(() => { fastify.close() })

    sget({
      method: 'POST',
      url: 'http://localhost:' + fastify.server.address().port,
      headers: { 'Content-Type': 'application/json' },
      body: [],
      json: true
    }, (err, response, body) => {
      t.error(err)
      t.equal(response.statusCode, 200)
    })
  })
})

…tta98/fastify into feature@request-bodylimit

debadutta98 · 2022-11-05T13:05:38Z

Can we modify the test in request-error.js to this please? Just in case we have in future some routeOption being an Object or Array, we could overlook, that Object.freeze does not cover them properly.

Thank you!!

Your solution has been implemented

climba03003 · 2022-11-05T13:40:21Z

Object.freeze does not cover them properly.

Probably the one would be constraints.

github-actions · 2023-11-06T00:21:10Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

debadutta98 added 3 commits November 3, 2022 00:59

add a new routeBodyLimit to request object

b2c6290

change testcase port number

7bd4d00

Merge branch 'fastify:main' into feature@request-bodylimit

0235806

mcollina approved these changes Nov 2, 2022

View reviewed changes

climba03003 reviewed Nov 3, 2022

View reviewed changes

docs/Reference/Request.md Outdated Show resolved Hide resolved

move routeBodyLimit under routeConfig on Request.md

82c0f2f

Eomm requested changes Nov 3, 2022

View reviewed changes

Eomm reviewed Nov 3, 2022

View reviewed changes

docs/Reference/Request.md Outdated Show resolved Hide resolved

Eomm added the semver-minor Issue or PR that should land as semver minor label Nov 3, 2022

debadutta98 added 2 commits November 3, 2022 13:14

update testcase and update request.js

8662f75

change the bodylimit property path to request.routeOptions.bodyLimit

00a3819

jsumners reviewed Nov 3, 2022

View reviewed changes

docs/Reference/Request.md Show resolved Hide resolved

docs/Reference/Request.md Outdated Show resolved Hide resolved

update request.md

c92f9f6

jsumners reviewed Nov 3, 2022

View reviewed changes

lib/request.js Outdated Show resolved Hide resolved

remove double parenthesis

fb5032c

jsumners requested changes Nov 3, 2022

View reviewed changes

mcollina approved these changes Nov 3, 2022

View reviewed changes

debadutta98 added 2 commits November 4, 2022 10:45

- Add more options to request.routeOptions - Change the types - Updat…

cb8dce7

…e testcases

update Request.md

ac665ab

climba03003 requested changes Nov 4, 2022

View reviewed changes

lib/request.js Outdated Show resolved Hide resolved

lib/request.js Outdated Show resolved Hide resolved

lib/request.js Outdated Show resolved Hide resolved

update request.js

a149fa8

climba03003 requested changes Nov 4, 2022

View reviewed changes

lib/request.js Outdated Show resolved Hide resolved

lib/request.js Show resolved Hide resolved

jsumners reviewed Nov 4, 2022

View reviewed changes

lib/request.js Outdated Show resolved Hide resolved

debadutta98 added 2 commits November 4, 2022 17:21

add version property to route options

f87b119

replace readOnly with Object.freeze

81732f5

jsumners reviewed Nov 4, 2022

View reviewed changes

docs/Reference/Request.md Outdated Show resolved Hide resolved

debadutta98 added 2 commits November 4, 2022 17:46

update request.md

33817b4

update request.md with some changes

6f16f02

debadutta98 added 3 commits November 4, 2022 19:30

exposeHeadRoute and prefixTrailingSlash to routeOptions

679d876

update request.md doc

501785f

Merge branch 'fastify:main' into feature@request-bodylimit

014dbad

jsumners approved these changes Nov 4, 2022

View reviewed changes

Eomm approved these changes Nov 5, 2022

View reviewed changes

Eomm changed the title ~~[Feature] Add routeBodyLimit to request~~ feat: add request.routeOptions object Nov 5, 2022

debadutta98 added 2 commits November 5, 2022 18:28

update request-error.test.js - request.routeOptions should be immutable.

c053b22

Merge branch 'feature@request-bodylimit' of https://github.com/debadu…

e3302f3

…tta98/fastify into feature@request-bodylimit

climba03003 approved these changes Nov 5, 2022

View reviewed changes

Uzlopak approved these changes Nov 5, 2022

View reviewed changes

mcollina merged commit f1bd80e into fastify:main Nov 5, 2022

metcoder95 mentioned this pull request Dec 7, 2022

Access handler name #4439

Closed

2 tasks

trentm mentioned this pull request Nov 1, 2023

FastifyDeprecation warning for accessing request.routerPath when accessing a non-existing page open-telemetry/opentelemetry-js-contrib#1757

Closed

github-actions bot locked as resolved and limited conversation to collaborators Nov 6, 2023

Uh oh!

Conversation

debadutta98 commented Nov 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Work

Checklist

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

climba03003 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

debadutta98 commented Nov 3, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jsumners left a comment

Choose a reason for hiding this comment

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

debadutta98 commented Nov 4, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Eomm left a comment

Choose a reason for hiding this comment

Uh oh!

Uzlopak commented Nov 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

debadutta98 commented Nov 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

climba03003 commented Nov 5, 2022

Uh oh!

github-actions bot commented Nov 6, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

debadutta98 commented Nov 2, 2022 •

edited

Loading

climba03003 left a comment •

edited

Loading

Uzlopak commented Nov 5, 2022 •

edited

Loading

debadutta98 commented Nov 5, 2022 •

edited

Loading