Symbol keys not handled in combination with additionalProperties

[jamiecobbett]

The following test fails, but I don't believe it should:

  def test_additional_properties_with_symbols_and_strings
    schema = {
      type: 'object',
      required: ["a"],
      properties: {
        "a" => {type: "integer", default: 42}
      },
      additionalProperties: false
    }

    data = {
      a: 5
    }

    assert(JSON::Validator.validate(schema, data))
  end

The immediate problem, I think, is in https://github.com/hoxworth/json-schema/blob/master/lib/json-schema/attributes/additionalproperties.rb#L37 which subtracts the keys in the data from the keys in the schema. This works when they are of the same type, but not when one is a symbol and one is a key.

The support in json-schema for supplying keys as symbols or strings seems to be difficult to make consistent. Can I suggest that support for symbols is removed, and instead users have to supply string keys? If they want to use symbols, they can call something like ActiveSupport's deep_stringify_keys. Alternatively, this library could add a dependency on ActiveSupport itself, or reimplement that method and apply it to parameters.

[pd]

👍 for stringifying keys upfront, and only using strings throughout the guts of the implementation.

I'm curious how often symbols are even useful outside of test suites (where it's more convenient to write the schema in ruby); wouldn't most of us be loading the schema we validate against from actual JSON, anyhow? In which case, everything's already a string. But based on the number of issues filed related to symbols, people must be using it somehow!

Personally, I think dropping support for symbols is The Right Thing ™️ -- but it breaks backwards compatibility and would necessitate a 3.x release. So stringifying seems like the only reasonable way forward.

[hoxworth]

Ugh, yes, symbol support has been the biggest bane of this library. The problem with removing support for it is that a lot of users make use of symbol keys, as @pd mentioned. Many users directly implement their schemas as ruby hashes, and want to follow standard ruby idioms in doing so. The new hash syntax in 1.9 really pushed a lot of people toward using symbol keys.

The keys should definitely be stringified upfront; this needs to be done ASAP. As for a longer-term removal of symbol keys, I'll mull that one over for a while and figure out the best course of action.

[apsoto]

Would using ActiveSupport::HashWithIndifferentAccess within the library 'fix' the problem? Users can continue to use strings or symbols, but it's all strings internally?

[hoxworth]

@apsoto That's what we're pretty much currently doing here. As @jamiecobbett points out, this approach doesn't work with the current additionalProperties approach of subtracting one hash's keys from another, without further modifications and hacks.

It would be cleaner to follow a standard convention (such as converting all keys to strings) and have the rest of the hash methods just work, as opposed to adding hacks to all of the hash methods to make them work with indifferent access.

I also really don't want to add activesupport as a dependency to this library, just to keep the dependency footprint as small as possible.

[dekellum]

I use JSON.parse( msg, symbolize_names: true ) frequently. It would be nice if we could reliably validate symbol-key hashes against a symbol-key schema, or string-key hashes against a string-key schema. I don't think its necessary to support mixing key types, particularly at the expense of performance (always full copy) or code complexity.

What if "indifferent access" and any other special handling was removed in a next MAJOR release? Then couldn't this issue be resolved by simply documenting that all symbols or all strings works, but not mixing?

[iainbeeston]

This should work now, as we do symbolize keys internally (at least on master - I don't think that change has been released as a gem yet).

[RST-J]

There is a 2.4.1 release from yesterday including this fix.

[iainbeeston]

Ah yes - great!