clearhaus / pedicel

Handle Apple Pay payment tokens.
MIT License
6 stars 3 forks source link

Use symbols in improve validator #22

Closed kse-clearhaus closed 6 years ago

kse-clearhaus commented 6 years ago

Replaced strings with symbols.

The only non-essential change is a change from parentheses to braces in symbol (%i[..]) arrays. Recommended by the style guide.

kse-clearhaus commented 6 years ago

Fair point. Thinking.

kse-clearhaus commented 6 years ago

Ah, yup, just had to wrap my head around it.

This is not an issue, the validator schema Dry::Validation::Schema::JSONis used here: https://github.com/clearhaus/pedicel/blob/4c413614dfde60e4ee94e39ea128d9355ea5003c/lib/pedicel/validator.rb#L62

It automatically symbolizes names:

https://github.com/dry-rb/dry-types/blob/fb19568f44c9086b6761c6e6f77be3021555d988/lib/dry/types/hash/schema_builder.rb#L32

in Dry::Validation here: https://github.com/dry-rb/dry-validation/blob/7d2231130fb08f5756ec285aad8885ab2cd93cab/lib/dry/validation/schema/json.rb#L10

I cannot find any documentation on this, I only remember seeing github issues on it.

kse-clearhaus commented 6 years ago

Another option would be to change as in this example:

describe 'Pedicel::Validator::TokenDataSchema' do
  let(:tds) { Pedicel::Validator::TokenDataSchema }
  let(:token_data_h) { JSON.parse(token.unencrypted_data.to_hash.to_json) }
  subject { token_data_h }

  context 'wrong data' do
    %w[
      applicationPrimaryAccountNumber
      applicationExpirationDate
      currencyCode
      transactionAmount
      deviceManufacturerIdentifier
      paymentDataType
      paymentData
    ].each do |required_key|
      it "errs when #{required_key} is missing", me: true do
        token_data_h.delete(required_key)
        # Not the '.to_sym' below
        is_expected.to dissatisfy_schema(tds, required_key.to_sym => ['is missing'])
      end
    end
ct-clearhaus commented 6 years ago

This allows hashes with both strings and symbols as keys. And that, I think, is a nice thing!

The only consequence I kinda dislike is the #error hash that will then always have symbols as keys which is counter-intuitive if you supply a hash with string keys to EC.new. Although it's probably most normal to supply string hashes (because that's what JSON.parse will return you unless you pass symbolize_names: true; likewise for Oj.load without symbol_keys: true)), at least all #errors keys are now symbols—since the high-level rules (because of dry-validation(*)) will be symbol keyed.

(*): Well, at least I couldn't make rule accept string keys for the rule name.

@kse-clearhaus 👀 🙂

kse-clearhaus commented 6 years ago

:eyes: :microscope:

kse-clearhaus commented 6 years ago

Merged manually.