Spec has a binary field - openapi-first failing with exception trying to get the :tempfile

[neongrau]

Field isn't in required so by default it is optional. Yet it's tried to be read.

data is the payload, property the name (image in my case) but it wasn't sent so data['image'] # nil

    def binary_format(data, property, property_schema, _parent)
      return unless property_schema.is_a?(Hash) && property_schema['format'] == 'binary'

      data[property] = data[property][:tempfile].read ### 
    end

[ahx]

@neongrau can you share (parts of) your API description to build a realistic test case?

[ahx]

Hi @neongrau I have added a fix in https://github.com/ahx/openapi_first/pull/241 (branch). Can you test this?

There are quite a few ways to describe file-uploads in OpenAPI, but I have not worked with any of them really. So I would like to know how people would like to use this feature.

[neongrau]

Hi @neongrau I have added a fix in #241 (branch). Can you test this?

There are quite a few ways to describe file-uploads in OpenAPI, but I have not worked with any of them really. So I would like to know how people would like to use this feature.

Hi and thanks for the fast reply!

Mostly my endpoints are used with just JSON:API data. For files, some smaller ones can be supplied as Base64 encoded. Though some endpoints would handle documents (PDF handbooks) that can be quite large and these are very memory intense to handle as Base64. So i ended with openapi specs that handle json as well as multipart-form-data.

                data[image]:
                  type: string
                  format: binary
                  writeOnly: true
                  description: multipart form data image to upload
                data[image_data_uri]:
                  type: string
                  writeOnly: true
                  description: base64 encoded image to upload

For real a full example see https://dev.samedis.care/api-docs/v4/public/index.html#?route=post-/api/v4/tenants/-tenant_id-/device_models/-device_model_id-/uploads

Will test your fix later today and give feedback.

[ahx]

FYI: Currently openapi_first reads the file to make the file contents available for json-schema validation and to make the file contents available at for example env[OpenapiFirst::REQUEST].body['data']['image']. I am wondering if anyone is using the latter at all when handling file uploads in form-data.

I am thinking about no longer reading the file contents in request validation and relax validating "binary" format by just checking if a file is uploaded, but reading the actual file during request validation. This would break the above interface, but it probably what people want most of the time. What do you think?

[neongrau]

FYI: Currently openapi_first reads the file to make the file contents available for json-schema validation and to make the file contents available at for example env[OpenapiFirst::REQUEST].body['data']['image']. I am wondering if anyone is using the latter at all when handling file uploads in form-data.

I am thinking about no longer reading the file contents in request validation and relax validating "binary" format by just checking if a file is uploaded, but reading the actual file during request validation. This would break the above interface, but it probably what people want most of the time. What do you think?

TBH i'm not sure what tests would make sense here. I don't think OpenAPI supports any logic like content-type or size restrictions. So currently all i can think of would be checking presence alone, though only when in the required array and not nullable if sent empty.

I just started looking into what openapi_first can do yesterday. Had to overcome a hurdle first by implementing a wrapping middleware that first determines which spec file to load as we recently had to split our specs into a bunch of spec files as they were getting too large and some are meant for external use (stable) and others only internal (changed whenever the frontend/features demand).

[ahx]

So currently all i can think of would be checking presence alone

Thanks for the input.

Had to overcome a hurdle first by implementing a wrapping middleware that first determines which spec file to load as we recently had to split our specs into a bunch of spec files.

Understood. Are you using Rails? We usually do this by mounting openapi_first middlewares inside controllers or base controllers for each API instead of adding one global middleware. Please don't hesitate to create an issue/discussion about this if you have a (vague) idea how this should be easier to use.

[neongrau]

Understood. Are you using Rails? We usually do this by mounting openapi_first middlewares inside controllers or base controllers for each API instead of adding one global middleware.

Yes Rails 7. Though could you give me an example how you use it within your controllers? As i said i just started and slapped together a middleware class to get around the multiple spec file problem. Might not be the best approach, but seems to work so far for request validation. Found a couple of minor issues with our specs already. Haven't even gotten to response validation yet.

Just had spend a couple weeks to move from a totally broken swagger spec to proper validating specs with OAS3.0 and from rswag UI to openapi-explorer.

[ahx]

Just had spend a couple weeks to move from a totally broken swagger spec to proper validating specs with OAS3.0 and from rswag UI to openapi-explorer.

That sounds painful.

I guess the most straightforward solution to go forward without any impact on production would be something along the lines of:

module InternalApi
  class ApplicationController < ActionController::Base
    if ENV['RACK_ENV'] == 'test'
      use OpenapiFirst::Middlewares::ResponseValidation, spec: 'openapi/internal_openapi.yaml'
      use OpenapiFirst::Middlewares::RequestValidation, spec: 'openapi/internal_openapi.yaml', raise_error: true
    end

    # ...
  end
end

And once you are green you can run OpenapiFirst::Middlewares::RequestValidation on production and OpenapiFirst::Middlewares::ResponseValidation in tests only.

[neongrau]

and btw... the fix seems to work.

[ahx]

Fix released in 1.3.3

[neongrau]

Fix released in 1.3.3

are you sure the fix is included? only seeing Gemfile and version changes

[ahx]

damn

[ahx]

Released before merge. Sorry. Fix released in 1.3.4