[FeatureRequest] Add convenience procs to check if a formparam/queryparam exists

[PhilippMDoerner]

Right now, prologue has getQueryParam / getPathParam / getPostParam / getFormParam. However, for my own usecases I have scenarios where the same controller handles multiple possible URLs / form input possibilities and I'd like to be able to just check whether a given request has a specific form param/pathParam/queryParam etc.

So convenience procs like "hasQueryParam" etc. would be very useful to me. Would it be desired to add such procs to prologue? Alternatively, the same could be achieved by having the "get" procs above return Option types, to symbolize that a given param might not exist.

[ringabout]

Adding an overloading proc returning option types seems to be a good idea. Then make the old one call the new overloading internally to maintain backward compatibility.

[PhilippMDoerner]

Adding an overload returning option types seems to be a good idea.

Ohhh, so the 2 signatures would look like this:

func getFormParams*(ctx: Context, key: string, default = ""): string {.inline.} =

func getFormParams*(ctx: Context, key: string): Option[string] {.inline.} =

But won't that still possibly run into issues because the default param in the original proc has a default parameter and thus could be omitted, giving both of them the same signature (if you ignore the output) ?

[ringabout]

Yeah, I think we can change the signature to this:

func getFormParamsOption*(ctx: Context, key: string): Option[string] {.inline.}

[PhilippMDoerner]

I assume you've already got enough on your plate. I'll look into making a PR when I've got the mental headspace, annoyingly enough, writing the tests will be likely 80% of the work on this thing.

[ringabout]

I assume you've already got enough on your plate.

Haha, yes. I'm planning writing a new frontend framework using Nim with proper UI supporting.

I'll look into making a PR when I've got the mental headspace, annoyingly enough, writing the tests will be likely 80% of the work on this thing.

Thanks, I think writing unittest for procs should be much simpler: https://github.com/planety/prologue/tree/devel/tests/unit/tunit_core

[PhilippMDoerner]

Just to be on the safe side, in order to run all tests on prologue I just need to run nimble tests in the root dir? Mostly asking because the project I previously contributed to (norm) had some extra bells and whistles due to tests involving a postgres db.

[ringabout]

in order to run all tests on prologue I just need to run nimble tests in the root dir

Only If you have installed all the deps the tests need

- name: Install Packages
  run: nimble install -y

- name: Install extension
  run: logue extension all

- name: Test
  run: nimble tests

tests depend on extra libraries

task redis, "Install redis":
  exec "nimble install redis@#c02d404 -y"

task karax, "Install karax":
  exec """nimble install karax@">= 1.1.2" -y"""

task websocketx, "Install websocketx":
  exec """nimble install websocketx@">= 0.1.2" -y"""

[PhilippMDoerner]

Hmmm it appears I can't build karax for the specific 1.1.2 version

~/dev/prologue % nimble install karax@">= 1.1.2" -y Downloading https://github.com/karaxnim/karax using git Warning: Package 'karax' has an incorrect structure. It should contain a single directory hierarchy for source files, named 'karax', but file 'tester.nim' is in a directory named 'tests' instead. This will be an error in the future. Hint: If 'tests' contains source files for building 'karax', rename it to 'karax'. Otherwise, prevent its installation by adding skipDirs = @["tests"] to the .nimble file. Verifying dependencies for karax@1.2.1 Info: Dependency on ws@any version already satisfied Verifying dependencies for ws@0.5.0 Info: Dependency on dotenv@any version already satisfied Verifying dependencies for dotenv@2.0.1 Installing karax@1.2.1 Building karax/karax/tools/karun using c backend /tmp/nimble_154165/githubcom_karaxnimkarax_1.1.2/karax/tools/static_server.nim(140, 13) Error: undeclared identifier: 'DotEnv' candidates (edit distance, scope distance); see '--spellSuggest': (1, 3): 'dotenv' [module declared in /tmp/nimble_154165/githubcom_karaxnimkarax_1.1.2/karax/tools/static_server.nim(6, 8)] Prompt: Build failed for 'karax@1.2.1', would you like to try installing 'karax@#head' (latest unstable)? -> [forced yes]

I think it still works though? When running nimble tests I get some talk about joining test files and then a single tserver_application.nim gets checked and passes.

JOINED: tests/assets/tassets.nim c joinable specs: 40 PASS: tests/server/tserver_application.nim c ( 0.29 sec) megatest output OK

Would that mean it is running all the tests and just concats them first into a single file (I would dare question as to why that is, that feels like going through extra-hoops)?

[ringabout]

I think it still works though? When running nimble tests I get some talk about joining test files and then a single tserver_application.nim gets checked and passes.

karax seems to need a release, maybe file a new issue. That's fine if tests are passed.

Would that mean it is running all the tests and just concats them first into a single file (I would dare question as to why that is, that feels like going through extra-hoops)?

testament all concatenates all the tests into a single file as far as possible because it reduces the compilation time massively.

[PhilippMDoerner]

Check. I see no tests for the procs beyond multimatch and some context stuff in tunit_context.nim. Shall I write tests or is the intention to keep this test-light for now for development flexibility in order to not commit to a specific API too much?

[ringabout]

I see no tests for the procs beyond multimatch and some context stuff in tunit_context.nim. Shall I write tests or is the intention to keep this test-light for now for development flexibility in order to not commit to a specific API too much?

Feel free to add tests for uncovered procs.

[PhilippMDoerner]

Err.... can I not use the unittest module? I just wrote a suite of tests and testament.all failed to bring this together. Am I forced to use block ?

-[Suite] getQueryParams

• [OK] Given the query param exists, When the form param is queried, Then return the form param megatest:processing: [22] tests/unit/tunit_core/tunit_encode.nim megatest:processing: [23] tests/unit/tunit_core/tunit_form.nim megatest:processing: [24] tests/unit/tunit_core/tunit_framework_config/tunit_framework_config.nim

FAIL: megatest output different, see outputGotten.txt vs outputExpected.txt stack trace: (most recent call last) /tmp/nimblecache-3313502514/nimscriptapi_478646173.nim(187, 16) /home/philipp/dev/prologue/prologue.nimble(21, 8) testsTask /home/philipp/.choosenim/toolchains/nim-1.6.4/lib/system/nimscript.nim(273, 7) exec /home/philipp/.choosenim/toolchains/nim-1.6.4/lib/system/nimscript.nim(273, 7) Error: unhandled exception: FAILED: testament all [OSError] Error: Exception raised during nimble script execution

[ringabout]

can I not use the unittest module? I just wrote a suite of tests and testament.all failed to bring this together. Am I forced to use block ?

unittest can be combined with testament, though it is not a necessity. block + doAssert is perferred.

Imo, you need to specify the output of stdout:

see https://github.com/nim-lang/Nim/blob/8ccde68f132be4dba330eb6ec50f4679e564efac/tests/stdlib/tunittest.nim#L2

see also https://nim-lang.org/docs/testament.html

[PhilippMDoerner]

unittest can be combined with testament, though it is not a necessity. block + doAssert is perferred.

If you can tolerate it I would very much prefer to stick with suite + test. The reason for that is, that with the string in "suite" I can designate which function is under unit test, while with the string for "test" I can denote the specific "Given - When - Then" relationship that I am testing and thus more aptly describe the expected behaviour. I can achieve something similar with block and using comments, but it'd be less clean comparatively imo.

[ringabout]

If you can tolerate it I would very much prefer to stick with suite + test.

Okay. suite + test or echo tests can probably pollute the error messages. That's why they are discouraged in Nim repo. I'm fine with it in the prologue tests. And it is better to specify joinable = false in the discard statement.

[PhilippMDoerner]

With the PR being merged, the necessary procs are now available. Wait... maybe I should put them also in the prologue book docs before closing this, hmmm

[ringabout]

sounds good