Add support for duplicate keys in parameters by adding paramValuesAsSeq

[torbencarstens]

Closes #247

I've implemented an example solution (new function) so that we can discuss further based on this.

I've opened this as a PR since the actual implementation discussion is not really relevant for the issue itself.

I'm sorry to say that I'm still not entirely sure how the following would work:

My suggestion was for a way to do it without changing jester-code, but just hooking in on available fields = the .url

I'm still fairly new to nim (<1000 lines) so forgive me if I'm missing something obvious here, the only options I see to implement this are

• experimental features (e.g. dotOperators)
• adding a url property (new type?) to Request which supports this feature (this modifies code even further though and would be inconsistent with the current implementation)
• probably templates/macros similar to promoting a proc to a different implementation when explicitely called by the user (or the library)
• implementing this for both httpbeast and the standard library and expose this via jester (also needs code changes since the NativeRequest isn't exposed to the user) (also unlikely to be accepted)

That being said, this should probably be a separate function in any case which can be used by any of the other options if another shortcut/accessor is implemented for this.

I'm not entirely happy with the name (feel free to suggest others) but I hope it's at least clear enough on what it does.

I've removed the explicit call to decodeUrl(value) (in both params and paramValuesAsSeq) since decodeData (decodeQuery internally) does the decoding for us already.

parseUrlQuery

I've also updated the params function to not use parseUrlQuery anymore since this has been marked as deprecated in any case.

With this change the 2 function operate essentially the same exact way (excluding the return type).

I haven't removed it since it's exported and might be used by users.

Tests

I've also implemented some tests for both params and paramValuesAsSeq (exactly the same tests with different results).

I tested this against the old version of params and the new version.

CI

os

Currently the CI:

• fails on windows
  • I don't really have access to a windows machine right now and have no idea what the problem could be
• aborts on mac due to the runner coming up late
• succeeds on linux

versions

CI succeeds for nim 1.4.8 and 1.6.14, for nim 2 the CI throws a SIGSEGV: Illegal storage access. (Attempt to read from nil?).

I've observed this behaviour with jester before and either running nim with --mm:refc or importing std/segfaults fixed this (which is very frustrating).

I've only tested running with --mm:refc which didn't fix it in the CI, I didn't want to import std/segfaults in all tests only for the CI to work since this is obviously a bug somewhere.

I haven't looked into this any closer though, everything (tests + jester itself) run fine for me with nim 2.0 (locally and in docker).

---

Comments on things I've updated which aren't directly related to the issue. I hope including this here is fine with you, otherwise I don't mind pulling these changes out and opening a separate PR with them.

Exclude testing tests/nim-in-action-code for nim >= 2.0

nim-in-action-code isn't trying to be compatible with nim 2 (as far as I can tell).

The compilation currently fails because db_sqlite isn't available anymore (needs a nimble install db_connector and a change of import, i.e. -> db_connector/db_sqlite

Update the CI

Basically this #319, I didn't want a broken CI when submitting a PR.

Update nimble test

Up to now the user had to execute some steps themselves (e.g. refresh/install), nimble test should now work without any further input from the user after cloning the repository.

.gitignore

I've developed with intellij so I've added the common ignores for this IDE

has been reverted (see #323 (comment))

[ThomasTJdev]

Access url field

I'm sorry to say that I'm still not entirely sure how #247 (comment):

My suggestion was for a way to do it without changing jester-code, but just hooking in on available fields = the .url

My mistake - I was sure, that there were a .url field which was public available, so you could roll you own procs on top. That would also have allowed users to implement their own query-collector, whether it is a Table[string, seq[string]] or something else.

Failing CI on nim v2

This is when httpbeast is used. The httpbeast#head includes a fix, but it must be published with a new tag (dom96/httpbeast#91).

[torbencarstens]

CI

nim v2

Alright, thanks for the info. I guess the addition of this CI version can be done at a later point then.

Windows

Regarding the CI on windows, the exact error message is (nim: 1.6.14)

C:\Users\{user}\.nimble\pkgs\asynctools-#0e6bdc3ed5bae8c7cc9\asynctools\asyncpipe.nim(260, 16) Error: undeclared identifier: 'PCustomOverlapped'

I found the following issue in asynctools: cheatfate/asynctools#31 which seems to explain this.

How do you want to proceed with this?

---

Aside from these 2 issues, is there anything left which has to be addressed (additional reviews/changes/documentation/examples/...)?

I guess at least the CI workflow still has to be approved and we need to test this on the mac pipeline (needs a decision on how the current windows bug should be handled).

[ThomasTJdev]

The .gitignore has two new files, which might be due to your editor preference - they can be removed.

For the asynctools I dont know - I'm not running windows... sorry.

I like the PR and your additional tests 👍. But, I'm just a contributor to the repo, so it must be @dom96 who can give you the directions for the next step.

[torbencarstens]

Alright, thanks for your help. I've reverted the .gitignore commit.

I'll wait for some input from dom then.

[torbencarstens]

Note about the Windows CI failures, I've fixed the asynctools issues locally but the tests still fail (explicitly import asyncpty and use asyncpty.PCustomOverlapped directly in asyncipc/asyncpipe).

I ran these on the current master to ensure that none of my changes has any impact on this. (I've ran mine as well and they're all [OK]).

The following test suites fail (I haven't looked into why this is):

• "static" / "index.html"
• "static" / "test_file.txt"
• "before/after" / "before - 404"

Note that the kill ${PID} message is a bit misleading due to the finally block error being directly underneath, the actual kill output comes from here https://github.com/dom96/httpbeast/blob/17e322ba68b264e9f4ec755e6946c64242d289ad/tests/tester.nim#L32 and is thrown in the allTest(useStdLib=true) testsuite when startServer is called.

I guess the impact from the Windows CI failing should be ignored in this case since it fails due to conditions which are already on master, this should probably be fixed separately.

logs for nim c --run tests/tester.nim (nim 1.6.14)

Hint: used config file 'C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\config\nim.cfg' [Conf]
Hint: used config file 'C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\config\config.nims' [Conf]
Hint: used config file 'C:\Users\{user}\projects\jester\tests\config.nims' [Conf]
Hint: gc: refc; opt: none (DEBUG BUILD, `-d:release` generates faster code)
19512 lines; 0.028s; 16.57MiB peakmem; proj: C:\Users\{user}\projects\jester\tests\tester.nim; out: C:\Users\{user}\projects\jester\tests\tester.exe [SuccessX]
Hint: C:\Users\{user}\projects\jester\tests\tester.exe  [Exec]
  Verifying dependencies for jester@0.6.0
  Compiling tests\alltest (from package jester) using c backend
C:\Users\{user}\projects\jester\jester\private\utils.nim(23, 18) Warning: inherit from a more precise exception type like ValueError, IOError or OSError. If these don't suit, inherit from CatchableError or Defect. [InheritFromException]
C:\Users\{user}\projects\jester\jester\private\utils.nim(41, 37) Warning: `typed` will change its meaning in future versions of Nim. `void` or no return type declaration at all has the same meaning as the current meaning of `typed` as return type declaration. [Deprecated]
C:\Users\{user}\projects\jester\jester\private\utils.nim(103, 12) Warning: Deprecated since v1.4; it was more confusing than useful, use `[]=`; add is deprecated [Deprecated]
C:\Users\{user}\projects\jester\jester\request.nim(115, 7) Warning: use stdlib cgi/decodeData; parseUrlQuery is deprecated [Deprecated]
C:\Users\{user}\projects\jester\jester.nim(183, 23) template/generic instantiation of `async` from here
C:\Users\{user}\projects\jester\jester.nim(186, 8) Warning: use fileExists; existsFile is deprecated [Deprecated]
C:\Users\{user}\projects\jester\jester.nim(320, 27) template/generic instantiation of `async` from here
C:\Users\{user}\projects\jester\jester.nim(333, 8) Warning: use dirExists; existsDir is deprecated [Deprecated]
C:\Users\{user}\projects\jester\jester.nim(547, 72) Warning: `typed` will change its meaning in future versions of Nim. `void` or no return type declaration at all has the same meaning as the current meaning of `typed` as return type declaration. [Deprecated]
C:\Users\{user}\projects\jester\jester.nim(568, 34) Warning: `typed` will change its meaning in future versions of Nim. `void` or no return type declaration at all has the same meaning as the current meaning of `typed` as return type declaration. [Deprecated]
C:\Users\{user}\projects\jester\jester.nim(577, 75) Warning: `typed` will change its meaning in future versions of Nim. `void` or no return type declaration at all has the same meaning as the current meaning of `typed` as return type declaration. [Deprecated]
C:\Users\{user}\projects\jester\jester.nim(589, 36) Warning: `typed` will change its meaning in future versions of Nim. `void` or no return type declaration at all has the same meaning as the current meaning of `typed` as return type declaration. [Deprecated]
C:\Users\{user}\projects\jester\jester.nim(595, 58) Warning: `typed` will change its meaning in future versions of Nim. `void` or no return type declaration at all has the same meaning as the current meaning of `typed` as return type declaration. [Deprecated]
C:\Users\{user}\projects\jester\jester.nim(606, 33) Warning: `typed` will change its meaning in future versions of Nim. `void` or no return type declaration at all has the same meaning as the current meaning of `typed` as return type declaration. [Deprecated]
C:\Users\{user}\projects\jester\jester.nim(615, 47) Warning: `typed` will change its meaning in future versions of Nim. `void` or no return type declaration at all has the same meaning as the current meaning of `typed` as return type declaration. [Deprecated]
C:\Users\{user}\projects\jester\jester.nim(632, 19) Warning: `typed` will change its meaning in future versions of Nim. `void` or no return type declaration at all has the same meaning as the current meaning of `typed` as return type declaration. [Deprecated]
C:\Users\{user}\projects\jester\jester.nim(639, 34) Warning: `typed` will change its meaning in future versions of Nim. `void` or no return type declaration at all has the same meaning as the current meaning of `typed` as return type declaration. [Deprecated]
C:\Users\{user}\projects\jester\jester.nim(646, 34) Warning: `typed` will change its meaning in future versions of Nim. `void` or no return type declaration at all has the same meaning as the current meaning of `typed` as return type declaration. [Deprecated]
C:\Users\{user}\projects\jester\jester.nim(658, 19) Warning: `typed` will change its meaning in future versions of Nim. `void` or no return type declaration at all has the same meaning as the current meaning of `typed` as return type declaration. [Deprecated]
C:\Users\{user}\projects\jester\jester.nim(663, 33) Warning: `typed` will change its meaning in future versions of Nim. `void` or no return type declaration at all has the same meaning as the current meaning of `typed` as return type declaration. [Deprecated]
C:\Users\{user}\projects\jester\jester.nim(666, 34) Warning: `typed` will change its meaning in future versions of Nim. `void` or no return type declaration at all has the same meaning as the current meaning of `typed` as return type declaration. [Deprecated]
C:\Users\{user}\projects\jester\jester.nim(669, 50) Warning: `typed` will change its meaning in future versions of Nim. `void` or no return type declaration at all has the same meaning as the current meaning of `typed` as return type declaration. [Deprecated]
C:\Users\{user}\projects\jester\jester.nim(672, 38) Warning: `typed` will change its meaning in future versions of Nim. `void` or no return type declaration at all has the same meaning as the current meaning of `typed` as return type declaration. [Deprecated]
C:\Users\{user}\projects\jester\jester.nim(685, 39) Warning: `typed` will change its meaning in future versions of Nim. `void` or no return type declaration at all has the same meaning as the current meaning of `typed` as return type declaration. [Deprecated]
C:\Users\{user}\projects\jester\tests\alltest.nim(9, 19) Warning: inherit from a more precise exception type like ValueError, IOError or OSError. If these don't suit, inherit from CatchableError or Defect. [InheritFromException]
C:\Users\{user}\projects\jester\tests\alltest.nim(10, 23) Warning: inherit from a more precise exception type like ValueError, IOError or OSError. If these don't suit, inherit from CatchableError or Defect. [InheritFromException]
C:\Users\{user}\projects\jester\tests\alltest.nim(29, 1) template/generic instantiation of `routes` from here
C:\Users\{user}\projects\jester\jester.nim(1352, 35) template/generic instantiation of `async` from here
C:\Users\{user}\projects\jester\tests\alltest.nim(44, 10) Warning: unreachable code after 'return' statement or '{.noReturn.}' proc [UnreachableCode]
C:\Users\{user}\projects\jester\tests\alltest.nim(29, 1) template/generic instantiation of `routes` from here
C:\Users\{user}\projects\jester\jester.nim(1352, 35) template/generic instantiation of `async` from here
C:\Users\{user}\projects\jester\tests\alltest.nim(143, 10) Warning: unreachable code after 'return' statement or '{.noReturn.}' proc [UnreachableCode]
C:\Users\{user}\projects\jester\tests\alltest.nim(6, 8) Warning: imported and not used: 'alltest_router2' [UnusedImport]
C:\Users\{user}\projects\jester\tests\alltest.nim(3, 53) Warning: imported and not used: 'asyncnet' [UnusedImport]
   Success: Execution finished
Getting http://localhost:5454 - attempt 0
Process: INFO Jester is making jokes at http://127.0.0.1:5454/foo
Process: ERROR C:\Users\{user}\projects\jester\tests\alltest.nim(29) alltest
Process: C:\Users\{user}\projects\jester\jester.nim(545) serve
Process: C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\pure\asyncdispatch.nim(1956) runForever
Process: C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\pure\asyncdispatch.nim(1653) poll
Process: C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\pure\asyncdispatch.nim(419) runOnce
Server started!
Process: C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\pure\asyncdispatch.nim(234) processPendingCallbacks
Process: C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\pure\asyncmacro.nim(28) serveNimAsyncContinue
Process: C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\pure\asynchttpserver.nim(425) serveIter
Process: C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\pure\asyncmacro.nim(232) processClient
Process: C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\pure\asyncmacro.nim(28) processClientNimAsyncContinue
Process: C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\pure\asynchttpserver.nim(362) processClientIter
Process: C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\pure\asyncmacro.nim(232) processRequest
Process: C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\pure\asyncmacro.nim(31) processRequestNimAsyncContinue
Process: C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\pure\asynchttpserver.nim(330) processRequestIter
Process: C:\Users\{user}\projects\jester\jester.nim(539) :anonymous
Process: C:\Users\{user}\projects\jester\jester.nim(399) handleRequest
Process: C:\Users\{user}\projects\jester\jester\request.nim(172) pathInfo
Process: C:\Users\{user}\projects\jester\jester\request.nim(166) stripAppName
Process: Expected script name at beginning of path. Got path: / script name: /foo
Process: DEBUG   502 Bad Gateway {"content-type": @["text/html;charset=utf-8"]}
Process: ERROR C:\Users\{user}\projects\jester\tests\alltest.nim(29) alltest
Process: C:\Users\{user}\projects\jester\jester.nim(545) serve
[OK] doesn't crash on missing script name
Process: C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\pure\asyncdispatch.nim(1956) runForever
Process: C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\pure\asyncdispatch.nim(1653) poll
Process: C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\pure\asyncdispatch.nim(419) runOnce
Process: C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\pure\asyncdispatch.nim(234) processPendingCallbacks
Process: C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\pure\asyncmacro.nim(28) serveNimAsyncContinue
Process: C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\pure\asynchttpserver.nim(425) serveIter
Process: C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\pure\asyncmacro.nim(232) processClient
Process: C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\pure\asyncmacro.nim(28) processClientNimAsyncContinue
Process: C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\pure\asynchttpserver.nim(362) processClientIter
Process: C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\pure\asyncmacro.nim(232) processRequest
Process: C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\pure\asyncmacro.nim(31) processRequestNimAsyncContinue
Process: C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\pure\asynchttpserver.nim(330) processRequestIter
Process: C:\Users\{user}\projects\jester\jester.nim(539) :anonymous
Process: C:\Users\{user}\projects\jester\jester.nim(399) handleRequest
Process: C:\Users\{user}\projects\jester\jester\request.nim(172) pathInfo
Process: C:\Users\{user}\projects\jester\jester\request.nim(166) stripAppName
Process: Expected script name at beginning of path. Got path: / script name: /foo
[OK] can access root
[OK] /nil
[OK] /halt
[OK] /halt-before
[OK] /guess
[OK] /redirect
[OK] /redirect-halt
[OK] /redirect-before
[OK] regex
[OK] resp
[OK] template
[OK] json
[OK] sendFile
[OK] can access query
[OK] can access querystring
[OK] issue 157
[OK] resp doesn't overwrite headers

[Suite] static
    C:\Users\{user}\projects\jester\tests\tester.nim(169, 27): Check failed: body.startsWith("This should be available at /root/.")
  [FAILED] index.html
    C:\Users\{user}\projects\jester\tests\tester.nim(173, 32): Check failed: (waitFor resp.body) == "Hello World!"
    (waitFor resp.body) was <html xmlns="http://www.w3.org/1999/xhtml"><head><title>400 Bad Request</title></head><body style="text-align: center;"><h1>400 Bad Request</h1><hr/><p>Jester 0.6.0</p></body></html>
  [FAILED] test_file.txt
  [OK] detects attempts to read parent dirs

[Suite] extends
  [OK] simple
  [OK] params
  [OK] separate module
  [OK] external regex
  [OK] regex path prefix escaped

[Suite] error
  [OK] exception
  [OK] HttpCode handling
  [OK] `pass` in error handler
    C:\Users\{user}\projects\jester\tests\tester.nim(220, 32): Check failed: (waitFor resp.body) == "404 not found!!!"
    (waitFor resp.body) was <html xmlns="http://www.w3.org/1999/xhtml"><head><title>400 Bad Request</title></head><body style="text-align: center;"><h1>400 Bad Request</h1><hr/><p>Jester 0.6.0</p></body></html>
  [FAILED] custom 404

[Suite] before/after
  [OK] before - halt
  [OK] before - unaffected
  [OK] before - global
    C:\Users\{user}\projects\jester\tests\tester.nim(237, 22): Check failed: resp.code == Http404
    resp.code was 400 Bad Request
    Http404 was 404 Not Found
  [FAILED] before - 404
  [OK] after - added
kill: 17728: No such process
kill: 17728: No such process
C:\Users\{user}\projects\jester\tests\tester.nim(296) tester
C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\system\assertions.nim(38) failedAssertImpl
C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\system\assertions.nim(28) raiseAssert
C:\Users\{user}\.choosenim\toolchains\nim-1.6.14\lib\system\fatal.nim(54) sysFatal
Error: unhandled exception: C:\Users\{user}\projects\jester\tests\tester.nim(296, 14) `execCmd("kill -15 " & $serverProcess.processID()) == QuitSuccess`  [AssertionDefect]
Error: execution of an external program failed: 'C:\Users\{user}\projects\jester\tests\tester.exe '