Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed JSON request had 1 or 0 array elements and add json_parameters method #21

Merged
merged 6 commits into from
Nov 9, 2023

Conversation

kfly8
Copy link
Collaborator

@kfly8 kfly8 commented Nov 8, 2023

This pull request solves the following problems.

Problem

When requesting JSON with one or zero array elements, the body parameters are no longer an array:

Case: POST { b => [123] }
Then: { b => 123} 
Expected body parameters : { b => [123] }

Case: POST { b => [] }
Then: {} 
Expected body parameters : { b => [] }

The following test code reproduces this problem. t/100_bugs/002_a_few_parameters_in_json_body.t

Solutions

  1. Decode body parameters recursively.
  2. Added json_parameters method for easier handling of JSON requests.

@kfly8 kfly8 self-assigned this Nov 8, 2023
@kfly8 kfly8 requested a review from kazeburo November 8, 2023 08:41
@kfly8 kfly8 changed the title Fixed bug: JSON request had 1 or 0 array elements, Fixed JSON request had 1 or 0 array elements and add json_parameters method Nov 8, 2023
@@ -160,9 +197,7 @@ sub _query_parameters {
sub body_parameters_raw {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change reverts back to the v0.60 code.

Kossy/lib/Kossy/Request.pm

Lines 155 to 161 in 4191730

sub body_parameters_raw {
my $self = shift;
unless ($self->env->{'plack.request.body'}) {
$self->env->{'plack.request.body'} = Hash::MultiValue->new(@{$self->_body_parameters});
}
return $self->env->{'plack.request.body'};
}

subtest 'body_parameters' => sub {
my $p1 = $req->body_parameters->{'p1'};

# NOTE: There is no need to use `get_all` method to get all of the array values.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For application/x-www-form-urlencoded and
multipart/form-data, but not the get_all method for application/json.

I think this is fine for this test, but essentially, I consider this a non-intuitive behavior.

Therefore, I would like to add a method called json_parameters to simplify the handling of JSON requests.


=item json_parameters

Accessor to decoded JSON body parameters. It's B<NOT> Hash::MultiValue object.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the reason for using Hash::MultiValue is to handle Arrays.

If JSON can handle structures such as Arrays, so I prefer to not using Hash::MultiValue.

# )
# );

NOTE: Not need to set C<kossy.request.parse_json_body> to 1
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When using json_parameters accessor, this setting is no longer necessary for simplicity.

@kazeburo kazeburo merged commit 1e618ea into master Nov 9, 2023
13 checks passed
@kfly8 kfly8 deleted the fix-bug-json-request branch November 9, 2023 05:19
kazeburo added a commit that referenced this pull request Nov 9, 2023
Changelog diff is:

diff --git a/Changes b/Changes
index 2f85f96..e638eaf 100644
--- a/Changes
+++ b/Changes
@@ -2,6 +2,10 @@ Revision history for Perl extension Kossy
 
 {{$NEXT}}
 
+0.62 2023-11-09T08:56:51Z
+
+        - Fixed JSON request #21
+
 0.61 2023-11-06T14:26:54Z
 
         - Enhance Kossy::Request to support array values in JSON body params #19 Thank you kfly8
kazeburo added a commit that referenced this pull request Nov 13, 2023
Changelog diff is:

diff --git a/Changes b/Changes
index e638eaf..4605acb 100644
--- a/Changes
+++ b/Changes
@@ -2,6 +2,10 @@ Revision history for Perl extension Kossy
 
 {{$NEXT}}
 
+0.63 2023-11-13T02:24:23Z
+
+        - Stabilize routing #22
+
 0.62 2023-11-09T08:56:51Z
 
         - Fixed JSON request #21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants