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

empty collection defaults break PATCH functionality #5

Closed
haywood opened this issue Mar 17, 2015 · 3 comments · Fixed by #6
Closed

empty collection defaults break PATCH functionality #5

haywood opened this issue Mar 17, 2015 · 3 comments · Fixed by #6

Comments

@haywood
Copy link
Contributor

haywood commented Mar 17, 2015

when specifying an operation like

                {
                    "method": "PATCH",
                    "path": "/:vendor_guid/:number",
                    "body": { "type": "item_patch" },
                    "description": "Upsert an item. Updates the item for the supplied vendor guid and number, creates it if one does not exist.",
                    "responses": {
                        "200": { "type": "item" },
                        "201": { "type": "item" },
                        "409": { "type": "[error]" }
                    }
                },

                },

given the below definition of item_patch

        "item_patch": {
            "description": "Used to PUT or PATCH an item.",

            "fields": [
                { "name": "quantity", "type": "long", "required": false },
                { "name": "prices", "type": "prices", "required": false },
                { "name": "attributes", "type": "[attribute]", "required": false },
                { "name": "return_policy", "type": "return_policy", "required": false },
                { "name": "metadata", "type": "[metadata]", "required": false },
                { "name": "identifiers", "type": "[identifier]", "required": false },
                { "name": "dimensions", "type": "[dimension]", "required": false },
                { "name": "content", "type": "[content]", "required": false },
                { "name": "images", "type": "[media]", "required": false },
                { "name": "videos", "type": "[media]", "required": false }
            ]
        },

one would expect that a field which is omitted from the patch would not be changed. However, due to the current default values for collection fields in both ruby and scala, the client generates empty collections that override the field to empty when sent to the server.

@haywood
Copy link
Contributor Author

haywood commented Mar 17, 2015

My proposal would be to generate Option[Coll[T]] in scala (e.g. Option[Seq[T]]) and default those fields to None (as we already do for Options). In ruby I would propose making the field nullable.

@haywood
Copy link
Contributor Author

haywood commented Mar 17, 2015

This issue is related to apicollective/apibuilder#293

@rcaloras
Copy link

👍

Didn't look like this included stuff for #7 does it? Want me to take a crack at that?

haywood pushed a commit that referenced this issue Mar 26, 2015
The implication of required is now equivalent to saying that
a given field must always be present when a model is sent
over the wire. Fields with defaults are required fields
for which generated code provides a default value.

All non-required fields are represented as options in Scala.

All non-required fields are represented as nullable in Ruby.

Non-required fields in Scala are now serialized using
writeNullable so that they are not mapped to null in
the JSON representation (#7).

Notably, non-required collections now map to Option in Scala
and nullable in Ruby. They no longer default to an empty instance
of the collection.

Default values are parsed and injected as language specific
literals client side.

It is illegal to specify a default for a non-required
field.

Fixes #5
Fixes #7
gheine pushed a commit that referenced this issue Mar 27, 2019
* add Postman Collection Generator base

* fail if service.imports are not resolved

* add postman models

* add ExampleJson along with its dependencies

* add postman generator heuristics along with its unit tests

* add service imports resolver and merger based on api-examples-poc repo

* first, rough, but working version of postman collection generator

* clean up the code a bit, add initial generator tests

* Add generated models (#1)

use apibuilder generated postman collection models

* extract PostmanItemBuilder, add the most important tests (#2)

* extract the main Operation mapping logic to PostmanItemBuilder. Add a wide set of the most important tests

* use parsed model in PostmanCollectionGenerator tests, apply review fixes

* remove default flow base url (#3)

* remove default flow base url

* Basic auth from Apibuilder attribute (remove hardcoded one) (#4)

* basic auth from apibuilder attribute

* fixes after rebase, review

* setup/clean flow organization on apibuilder flag (#5)

* setup/clean flow organization on apibuilder flag

* cover ServiceImportResolver with unit test suite, fix 2 existing bugs (#6)

* cover ServiceImportResolver with unit test suite, fix 2 existing bugs

* DependantOperationResolver tests and fix for nested dependencies (#7)

* add postman-generator specific readme

* switch to Postman Generator Attributes from ApiBuilder spec (#8)

* switch to Postman Generator Attributes from ApiBuilder spec

* put the ApiBuilder spec into separate temporary folder, update local files to the newly pushed Flow org app

* delete the old file before renaming, rename imports etc

* path variable dependent object handling (#9)

* Entities Setup payload fix, test and additional logging (#11)

* add Postman test assertions spec (#10)

* value substitue attribute + bugfix (#12)

* simplify object-reference attribute model, clean up the postman variable setting (#13)

* ServiceImportResolver refactor (#14)

* dynamic Entities Cleanup steps (#15)

* first version of dynamic cleanup steps ready

* improve delete operation path description

* remove hardcodes related to Flow

* address code review comments

* introduce ability to add query params and reference GET endpoints using `object-reference` (#16)

* first working version that was tested only manually. unit tests still to fix

* unit tests fixed

* extract Setup and Cleanup folders builder to a separate code

* DependantOperationResolver tests done. SetupCleanupFolderBuilder tests placeholder

* optimizations + SetupCleanupFolder tests

* fix issues pointed out in the review

* update local apibuilder generated model

* undeterministic random string generator in runtime (#17)

* undeterministic random string generator in runtime

* lorem ipsum connected by dash

* postman generator attributes format updated

* switch to ApiCollective specifications

* add some javadoc, extract common code to PathParamsFinder, remove TODO
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 a pull request may close this issue.

2 participants