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

fix(execute): parse stringified objects nested in arrays #3466

Merged
merged 2 commits into from
Apr 11, 2024

Conversation

glowcloud
Copy link
Contributor

Refs swagger-api/swagger-ui#9521

The URL for this example:

Screenshot 2024-04-10 at 14 36 22

should now be encoded as:

http://localhost:3200/endpoint?param=%5B%7B%22a%22%3A%7B%22c%22%3A%22string%22%7D%2C%22b%22%3A0%7D%5D

which should decode to:

http://localhost:3200/endpoint?param=[{"a":{"c":"string"},"b":0}]

@glowcloud
Copy link
Contributor Author

We're also doing a similar thing for parameters with schema and type object before going into builders:

if (
specIsOAS3 &&
parameter.schema &&
parameter.schema.type === 'object' &&
typeof value === 'string'
) {
try {
value = JSON.parse(value);
} catch (e) {
throw new Error('Could not parse object parameter value string as JSON');
}
}

I was thinking of moving the parsing for arrays there as well, and I think it would have to look something like this:

if (
  specIsOAS3 &&
  parameter.content &&
  parameter.content['application/json'] &&
  parameter.content['application/json'].schema?.type === 'array' &&
  Array.isArray(value)
) {
  value = value.map((v) => {
    try {
      return JSON.parse(v);
    } catch (e) {
      return v;
    }
  });
}

so not sure if having this would actually be better.

@glowcloud
Copy link
Contributor Author

I see that we can also refactor this line

return value.toString();

to

return String(value)

@char0n
Copy link
Member

char0n commented Apr 11, 2024

return String(value)

Yeah sure, it's small enough change to smuggle it with the bugfix.

@char0n
Copy link
Member

char0n commented Apr 11, 2024

I was thinking of moving the parsing for arrays there as well, and I think it would have to look something like this:

I was thinking the other way around, cannot we move the code from

if (
specIsOAS3 &&
parameter.schema &&
parameter.schema.type === 'object' &&
typeof value === 'string'
) {
try {
value = JSON.parse(value);
} catch (e) {
throw new Error('Could not parse object parameter value string as JSON');
}
}
to content-serializer to contain the logic there? If it's not possible, then placing your new code in
if (
specIsOAS3 &&
parameter.schema &&
parameter.schema.type === 'object' &&
typeof value === 'string'
) {
try {
value = JSON.parse(value);
} catch (e) {
throw new Error('Could not parse object parameter value string as JSON');
}
}
should be just fine.

@glowcloud
Copy link
Contributor Author

I was thinking of moving the parsing for arrays there as well, and I think it would have to look something like this:

I was thinking the other way around, cannot we move the code from

if (
specIsOAS3 &&
parameter.schema &&
parameter.schema.type === 'object' &&
typeof value === 'string'
) {
try {
value = JSON.parse(value);
} catch (e) {
throw new Error('Could not parse object parameter value string as JSON');
}
}

to content-serializer to contain the logic there? If it's not possible, then placing your new code in

if (
specIsOAS3 &&
parameter.schema &&
parameter.schema.type === 'object' &&
typeof value === 'string'
) {
try {
value = JSON.parse(value);
} catch (e) {
throw new Error('Could not parse object parameter value string as JSON');
}
}

should be just fine.

We can't move this to content-serializer as that's only for parameters with content, while this one is for parameters with schema. And I think that changing this to combine both logics might also create unnecessary parsing for arrays with objects in schema.

Copy link
Member

@char0n char0n left a comment

Choose a reason for hiding this comment

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

LGTM. nicely done

@glowcloud glowcloud merged commit b1f2ee7 into master Apr 11, 2024
5 checks passed
@glowcloud glowcloud deleted the serialize-content-arrays branch April 11, 2024 13:23
swagger-bot pushed a commit that referenced this pull request Apr 11, 2024
## [3.26.8](v3.26.7...v3.26.8) (2024-04-11)

### Bug Fixes

* **execute:** parse stringified objects nested in arrays ([#3466](#3466)) ([b1f2ee7](b1f2ee7))
@swagger-bot
Copy link
Contributor

🎉 This PR is included in version 3.26.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants