Error when updating content type with validations

[jsdalton]

I'm getting an IOError when attempting to update an existing content type that has fields with validation on it. The error is emanating from pretty deep within HTTP::Client, when the parameters are being coerced as json. It appears that during the process of converting properties to a hash, the :validations property of the Validation object has an extra Validation on it which is getting missed.

So let me see if I can explain. This is what fields looks like on an example content type I'm building at the create step:

[#<Contentful::Management::Field: @properties={
:id=>"featuredImage", 
:name=>"Featured Image", 
:type=>"Link", 
:linkType=>"Asset", 
:validations=>
[#<Contentful::Management::Validation: @properties={:linkMimetypeGroup=>"image"}>]}>]

After the object is created, if I re-retrieve the content type and attempt to update it, this is what fields looks like:

[#<Contentful::Management::Field: @properties={
 :id=>"featuredImage", 
 :name=>"Featured Image", 
 :type=>"Link", 
 :linkType=>"Asset", 
 :items=>#<Contentful::Management::Field:>, 
 :required=>false, 
 :localized=>false, 
 :validations=>[
                <Contentful::Management::Validation: @properties={:in=>nil, :size=>nil, :present=>false, 
                :validations=>#<Contentful::Management::Validation:>, :regexp=>nil, :linkContentType=>nil, :range=>nil, 
                :linkMimetypeGroup=>"image", :linkField=>false}>
                ]}>
]

The biggest difference is the existence of the :validations property on the Validation object itself, which gets left in place and cannot be coerced into json.

It looks like some special handling needs to be added to account for this property, akin to the way those other properties are handled in the parse_value of the Field class.

Let me know if the above makes sense or you have any questions. I'm going to attempt to work around this issue in the meantime.

[jsdalton]

Just for reference this is my janky workaround for the time being, pending this issue being resolved.

Where content_type is a Contentful::Management::ContentType

      def clean_validations(content_type)
        return unless content_type.fields.kind_of? Array
        content_type.fields.each do |field|
          next unless field.validations.kind_of? Array
          field.validations.each do |validation|
            validation.validations = nil
          end
        end
      end

I have to run this on a content_type if I wish to update it after it has been retrieved.

[pxlpnk]

Could you post the code that you use to create the content type, field and validation?
Which version of the Gem are you using, could you also let me know what the version of the http gem is you are using?
I am currently failing in reproducing the issue.

[jsdalton]

Sure let's see. Firstly, I'm using contentful-management (0.6.0).

As far as the code goes, here is the actual code being executed that reproduces the bug: (The actual calls to the contentful management library are hidden behind helper methods)

    fields = [
      new_asset_field(
        id: 'featuredImage',
        name: 'Featured Image',
        validations: [
          new_validation(link_mimetype_group: 'image')
        ]
      ),
    ]
    product_content_type = contentful_space.content_types.create(
      name: 'Product',
      id: 'product',
      fields: fields
    )
    product_content_type = contentful_space.content_types.find('product')

    # Workaround for https://github.com/contentful/contentful-management.rb/issues/49
    #clean_validations(product_content_type)

    product_content_type.update(displayField: 'productName')

Here is the debug level output when the code above is executed:

I, [2015-02-16T16:12:37.412481 #7352]  INFO -- : {:request=>{:url=>"https://api.contentful.com/spaces/tqg5dwrity1m", :query=>nil, :header=>{"User-Agent"=>"RubyContentfulManagementGem/0.6.0", "Authorization"=>"Bearer TOKENTOKEN", "Content-Type"=>"application/vnd.contentful.management.v1+json", "Content-Length"=>0}}}
D, [2015-02-16T16:12:38.034343 #7352] DEBUG -- : {:response=>#<HTTP::Response/1.1 200 OK #<HTTP::Headers {"Accept-Ranges"=>"bytes", "Access-Control-Allow-Headers"=>"Accept,Accept-Language,Authorization,Cache-Control,Content-Length,Content-Range,Content-Type,DNT,Destination,Expires,If-Match,If-Modified-Since,If-None-Match,Keep-Alive,Last-Modified,Origin,Pragma,Range,User-Agent,X-Http-Method-Override,X-Mx-ReqToken,X-Requested-With,X-Contentful-Version,X-Contentful-Content-Type,X-Contentful-Organization,X-Contentful-Skip-Transformation", "Access-Control-Allow-Methods"=>"DELETE,GET,HEAD,POST,PUT,OPTIONS", "Access-Control-Allow-Origin"=>"*", "Access-Control-Expose-Headers"=>"Etag", "Access-Control-Max-Age"=>"1728000", "Cache-Control"=>"max-age=0", "Content-Type"=>"application/vnd.contentful.management.v1+json", "Date"=>"Mon, 16 Feb 2015 16:12:37 GMT", "Etag"=>"\"32924504e04fdef690cd931448628d4b\"", "Server"=>"nginx", "Status"=>"200 OK", "X-Contentful-Request-Id"=>"85f-271212428", "Content-Length"=>"469", "Connection"=>"keep-alive"}>>}
I, [2015-02-16T16:12:38.036391 #7352]  INFO -- : {:request=>{:url=>"https://api.contentful.com/spaces/tqg5dwrity1m/content_types", :query=>nil, :header=>{"User-Agent"=>"RubyContentfulManagementGem/0.6.0", "Authorization"=>"Bearer TOKENTOKEN", "Content-Type"=>"application/vnd.contentful.management.v1+json", "Content-Length"=>0}}}
D, [2015-02-16T16:12:38.473940 #7352] DEBUG -- : {:response=>#<HTTP::Response/1.1 200 OK #<HTTP::Headers {"Access-Control-Allow-Headers"=>"Accept,Accept-Language,Authorization,Cache-Control,Content-Length,Content-Range,Content-Type,DNT,Destination,Expires,If-Match,If-Modified-Since,If-None-Match,Keep-Alive,Last-Modified,Origin,Pragma,Range,User-Agent,X-Http-Method-Override,X-Mx-ReqToken,X-Requested-With,X-Contentful-Version,X-Contentful-Content-Type,X-Contentful-Organization,X-Contentful-Skip-Transformation", "Access-Control-Allow-Methods"=>"DELETE,GET,HEAD,POST,PUT,OPTIONS", "Access-Control-Allow-Origin"=>"*", "Access-Control-Expose-Headers"=>"Etag", "Access-Control-Max-Age"=>"1728000", "Cf-Space-Id"=>"tqg5dwrity1m", "Content-Type"=>"application/vnd.contentful.management.v1+json", "Date"=>"Mon, 16 Feb 2015 16:12:38 GMT", "Etag"=>"\"29f2c21be26360c424f617d8592cf6f9\"", "Server"=>"nginx", "X-Powered-By"=>"Express", "Content-Length"=>"97", "Connection"=>"keep-alive"}>>}
I, [2015-02-16T16:12:38.475141 #7352]  INFO -- : {:request=>{:url=>"https://api.contentful.com/spaces/tqg5dwrity1m/content_types/product", :query=>{:name=>"Product", :description=>nil, :fields=>[{:id=>"featuredImage", :name=>"Featured Image", :type=>"Link", :validations=>[{:linkMimetypeGroup=>"image"}], :linkType=>"Asset"}]}, :header=>{"User-Agent"=>"RubyContentfulManagementGem/0.6.0", "Authorization"=>"Bearer TOKENTOKEN", "Content-Type"=>"application/vnd.contentful.management.v1+json"}}}
D, [2015-02-16T16:12:39.263547 #7352] DEBUG -- : {:response=>#<HTTP::Response/1.1 201 Created #<HTTP::Headers {"Access-Control-Allow-Headers"=>"Accept,Accept-Language,Authorization,Cache-Control,Content-Length,Content-Range,Content-Type,DNT,Destination,Expires,If-Match,If-Modified-Since,If-None-Match,Keep-Alive,Last-Modified,Origin,Pragma,Range,User-Agent,X-Http-Method-Override,X-Mx-ReqToken,X-Requested-With,X-Contentful-Version,X-Contentful-Content-Type,X-Contentful-Organization,X-Contentful-Skip-Transformation", "Access-Control-Allow-Methods"=>"DELETE,GET,HEAD,POST,PUT,OPTIONS", "Access-Control-Allow-Origin"=>"*", "Access-Control-Expose-Headers"=>"Etag", "Access-Control-Max-Age"=>"1728000", "Cf-Space-Id"=>"tqg5dwrity1m", "Content-Type"=>"application/vnd.contentful.management.v1+json", "Date"=>"Mon, 16 Feb 2015 16:12:39 GMT", "Etag"=>"\"54aeb903905a1a3a6facae44eec15b05\"", "Server"=>"nginx", "X-Powered-By"=>"Express", "Content-Length"=>"856", "Connection"=>"keep-alive"}>>}
I, [2015-02-16T16:12:39.265408 #7352]  INFO -- : {:request=>{:url=>"https://api.contentful.com/spaces/tqg5dwrity1m/content_types/product", :query=>nil, :header=>{"User-Agent"=>"RubyContentfulManagementGem/0.6.0", "Authorization"=>"Bearer TOKENTOKEN", "Content-Type"=>"application/vnd.contentful.management.v1+json", "Content-Length"=>0}}}
D, [2015-02-16T16:12:39.696854 #7352] DEBUG -- : {:response=>#<HTTP::Response/1.1 200 OK #<HTTP::Headers {"Access-Control-Allow-Headers"=>"Accept,Accept-Language,Authorization,Cache-Control,Content-Length,Content-Range,Content-Type,DNT,Destination,Expires,If-Match,If-Modified-Since,If-None-Match,Keep-Alive,Last-Modified,Origin,Pragma,Range,User-Agent,X-Http-Method-Override,X-Mx-ReqToken,X-Requested-With,X-Contentful-Version,X-Contentful-Content-Type,X-Contentful-Organization,X-Contentful-Skip-Transformation", "Access-Control-Allow-Methods"=>"DELETE,GET,HEAD,POST,PUT,OPTIONS", "Access-Control-Allow-Origin"=>"*", "Access-Control-Expose-Headers"=>"Etag", "Access-Control-Max-Age"=>"1728000", "Cf-Space-Id"=>"tqg5dwrity1m", "Content-Type"=>"application/vnd.contentful.management.v1+json", "Date"=>"Mon, 16 Feb 2015 16:12:39 GMT", "Etag"=>"\"d480fea9ccc23f71c92cb5ba4bac2e62\"", "Server"=>"nginx", "X-Powered-By"=>"Express", "Content-Length"=>"856", "Connection"=>"keep-alive"}>>}
I, [2015-02-16T16:12:39.699298 #7352]  INFO -- : {:request=>{:url=>"https://api.contentful.com/spaces/tqg5dwrity1m/content_types/product", :query=>{:displayField=>"productName", :name=>"Product", :description=>"", :fields=>[{:id=>"featuredImage", :name=>"Featured Image", :type=>"Link", :linkType=>"Asset", :items=>nil, :required=>nil, :localized=>nil, :validations=>[{:validations=>#<Contentful::Management::Validation:>, :linkMimetypeGroup=>"image"}]}]}, :header=>{"User-Agent"=>"RubyContentfulManagementGem/0.6.0", "Authorization"=>"Bearer TOKENTOKEN", "Content-Type"=>"application/vnd.contentful.management.v1+json", "X-Contentful-Version"=>1}}}

And here is the relevant implementation of those helper methods:

        def contentful_space
          @contentful_space ||= begin
            # Don't actually need the client, just need to intitialize it
            management_client
            ::Contentful::Management::Space.find(Rails.configuration.contentful_space_id)
          end
        end

        def new_asset_field(id:, name:, **kwargs)
          kwargs[:type] = ::Contentful::Management::ContentType::LINK
          kwargs[:link_type] = 'Asset'
          new_field(id: id, name: name, **kwargs)
        end

        def new_field(id:, name:, type:, **kwargs)
          field = ::Contentful::Management::Field.new
          field.id = id
          field.name = name
          field.type = type
          kwargs.each do |kwarg, value|
            unless value.nil?
              field.send("#{kwarg}=", value)
            end
          end
          field
        end

        # Workaround for JSON error when coercing fields to JSON. Pending resolution
        # of https://github.com/contentful/contentful-management.rb/issues/49
        def clean_validations(contentful_obj)
          if contentful_obj.respond_to?(:fields) && contentful_obj.fields.kind_of?(Array)
            contentful_obj.fields.each do |field|
              clean_validations(field)
            end
          elsif contentful_obj.respond_to?(:validations) && contentful_obj.validations.kind_of?(Array)
            contentful_obj.validations.each do |validation|
              validation.validations = nil
            end
          end
        end

        def management_client
          ::Contentful::Management::Client.shared_instance || ::Contentful::Management::Client.new(
            Rails.application.secrets.contentful['management_api_oauth_access_token'],
            :logger => Logger.new(STDOUT),
            :log_level => Logger::DEBUG,
          )
        end

I've commented out the clean_validations method above to create the error. When I run that, I'm able to proceed without error.

I would also draw your attention to the last request output in the logs I posted. If you look into the :query key of the reqeust, you can see there is a Validation object that has not been converted into a hash getting passed directly to in the query. This is what the HTTP client seems to be choking on when making the request.

Let me know if you have thoughts or suggestions on the above. Thanks!

[jsdalton]

I did some more work related to this issue today, and one question that arose for me is why a :validations property even makes sense on a Validation object? Digging in to the API response, there is no validations field returned under validations nor is there any concept of a validation having nested validations.

It's quite possible that I'm unaware of functionality or contexts where a nested validation might actually be used, but this seems to be the core of the issue. I've found, for example, that as a workaround to the fix I posted above I can simply remove the validations property from the Validation class and thereafter I do not have this error on update requests.

I hope this is helpful...

[pxlpnk]

Thank you for all the reports. I will take a look into them as soon as I am back from sick leave!

[pxlpnk]

I just tried to reproduce the issue with a bit of simpler code. Unfortunately I can not reproduce the error.
Could you have a look on what I might be missing?

require 'contentful/management'

client = Contentful::Management::Client.new(ENV['token'])
space = Contentful::Management::Space.find(ENV['space'])

validation = Contentful::Management::Validation.new
validation.link_mimetype_group = 'image'

title = Contentful::Management::Field.new
title.id = 'asset_title'
title.name = 'asset title'
title.type = 'Text'

field = Contentful::Management::Field.new
field.id = 'asset_field'
field.name = 'asset'
field.type = 'Link'
field.link_type = 'Asset'
field.validations = [validation]

space.content_types.create(name: 'ct-type', id: 'ct_type', fields: [title, field])

ct = space.content_types.find('ct_type')
ct.update(displayField: 'asset_title') # Here should be a breakage

What version of the http Gem are you using (bundle show http)?

[jsdalton]

Thanks for looking into this! I ran your exactly code above and was pretty mystified to as to why this would not reproduce the error we saw. I ended up firing both my original code and this script in two separate debugging sessions side by side, and stepping deep within the request building process to figure out what was going wrong.

I FINALLY got to the bottom of this -- and ironically it's literally the exact same root cause of a different bug that I reported in the main contentful gem here: contentful/contentful.rb#53

If you scroll up to the code I was using, specifically the helper method I have to create a client management_client you'll notice I'm instantiating the client with an instance of the Rails logger. This is the culprit!

What's happening is that when the validations key is being converted to JSON, it hits that sub validation object, and looks for a :to_hash method. It fails to find that method, so as a fall back it walks through the instance_values of the object. There are a ton of instance variables here, INCLUDING @logger!

In my example, @logger is the Rails logger, which throws an error when one tries to coerce it to JSON. Again, this is similar to what I experienced in the other gem.

Your example doesn't have the Rails logger, so it runs cleanly. I imagine if you have a rails project lying around you could pretty easily reproduce the actual error by feeding your client a Rails logger.

All this said, I'd definitely encourage you guys to consider the purpose of the :validations property. It doesn't really belong in the JSON request to the API and it's sort of "luck" that in the normal use case, the fallback method of walking through instance values doesn't cause an error. I'm having fine luck now just straight up disabling the property:

        ::Contentful::Management::Validation.class_eval do
          # Remove unnecessary field
          # Workaround for https://github.com/contentful/contentful-management.rb/issues/49
          property_coercions[:validations] = nil
          remove_method :validations
          remove_method :validations=
        end

But you might want to consider whether it belong as I mentioned previously.

[pxlpnk]

Which version of rails are you using? I tried to reproduce it now with the logger from the stdlib and an old rails 3.2 that I had lying around and could not reproduce it either.

[pxlpnk]

Ok I was now able to reproduce this.
I still do not understand where this sub validation object is coming from though.
I will investigate this a bit more. Thank you already for the great help! 👍