dry-rb/chat - Gitter

Piotr Solnica

@solnic

Aug 22 2018 07:29

@hderms wdym that dry-struct is logging sensitive info?

there's no logging functionality in dry-struct

Pablo Crivella

@pablocrivella

Aug 22 2018 07:31

Hi everyone, i’m experiencing an issue with the defines method of the dry-core gem, but i’m not sure if its actually a bug.
I have the following code (simplyfied), and what i’m trying to achieve is to transform keys according to a mapping defined on the class

require "dry/struct"
require "dry/core/class_attributes"

class Type < Dry::Struct
  extend Dry::Core::ClassAttributes

  defines :mappings

  mappings({})

  transform_keys { |key| reverse_mapping_for(key.to_sym) }

  class << self
    def reverse_mapping_for(attribute)
      mappings.key(attribute) || attribute
    end
  end
end

class Resource < Type
  defines :resource_name
end

class Relation < Resource
  resource_name "relaties"
  mappings(type: :relatiesoort, name: :naam)

  attribute :id, Types::Strict::String
  attribute :type, Types::Array(Types::Strict::String)
  attribute :name, Types::Strict::String
  attribute :email, Types::Strict::String
end

# NOTE: note the difference between “naam" and “name"
Relation.new(id: "1", type: ["Klant"], naam: "Rick", email: "rick@example.com")
# mappings returns an empty hash when called inside the `reverse_mapping_for` method
# Dry::Struct::Error: [Relation.new] :name is missing in Hash input


# If i move the `transform_keys` method call to the Relation class it works:

class Relation < Resource
  transform_keys { |key| reverse_mapping_for(key.to_sym) }
  resource_name "relaties"
  mappings(type: :relatiesoort, name: :naam)

  attribute :id, Types::Strict::String
  attribute :type, Types::Array(Types::Strict::String)
  attribute :name, Types::Strict::String
  attribute :email, Types::Strict::String
end

# NOTE: note the difference between “naam" and “name"
Relation.new(id: "das", type: ["Klant"], naam: "test", email: "asdsa")
=> #<Relation id="das" type=["Klant"] name="test" email="asdsa" establishment_address=nil>

Nikita Shilnikov

@flash-gordon

Aug 22 2018 08:01

@pablocrivella that's not a bug in dry-core, the block you pass to transform_keys is bound to Type, that's why reverse_mapping_for is always called on this object

you may want to use inherited to work around this, you'll need to call transform_keys in every subclass so that the context is not bound to Type

Pablo Crivella

@pablocrivella

Aug 22 2018 08:04

Thanks @flash-gordon ! I’ll go the inherited approach

Pablo Crivella

@pablocrivella

Aug 22 2018 08:09

      def inherited(subclass)
        puts subclass
        subclass.transform_keys { |key| reverse_mapping_for(key.to_sym) }
        super
      end

Something like this? Bc now a get a different error: “dry/struct/class_interface.rb:186:in transform_keys': undefined methodwith_key_transform' for nil:NilClass”

Nikita Shilnikov

@flash-gordon

Aug 22 2018 08:10

call super first

and subclass.reverse_mapping_for

Pablo Crivella

@pablocrivella

Aug 22 2018 08:11

Awesome, it works now, thanks so much for your help!

Tim Riley

@timriley

Aug 22 2018 08:17

@solnic I think @hderms might mean in the exception message that is output when trying to instantiate a struct with invalid data?

Piotr Solnica

@solnic

Aug 22 2018 08:51

I see, if that's the case, then it's not something a library like dry-struct should handle

need more context to provide recommendations

Dermot Haughey

@hderms

Aug 22 2018 14:48

@solnic i mean like the error message it constructs has the value inside a string. We want to log that message but how would we do so without logging sensitive info, i.e. the value

Piotr Solnica

@solnic

Aug 22 2018 14:59

you gotta handle this yourself

Dermot Haughey

@hderms

Aug 22 2018 15:00

how would you recommend we handle it ourselves

here is the code I'm describing: https://github.com/dry-rb/dry-struct/blob/master/lib/dry/struct/class_interface.rb#L201-L209

Nikita Shilnikov

@flash-gordon

Aug 22 2018 15:03

by rescuing errors and sanitizing them

Grant Shangreaux

@gcentauri

Aug 22 2018 15:08

you can configure your logger formatter to detect and sanitize things in the output too

Jeff Dickey

@jdickey

Aug 22 2018 18:21

Hey, @solnic, I just re-read the slides from your Architecture: The Reclaimed Years presentation on SlideShare; any chance there's a video of the talk around somewhere? I've a few teammates who would also find that interesting. Just asking…

Tim Mertens

@tmertens

Aug 22 2018 18:32

Hey @solnic I am trying to conditionally use a different schema for validation based on a feature flag. Do you have any recommendations on how to do this with dry-validations? I've tried a number of different approaches but am new to it and haven't been able to get it working

e.g.

rule(some_rule: [:type]) do |type|
  type.included_in?(VALID_TYPES).then(
    required(:other_field).schema(SomeFeature.enabled? ? SchemaOne : SchemaTwo)
  )
end

except this will evaluate the feature flag when building the AST, not when evaluating the rule for a given set of inputs. For example, in testing I want to be able to test w/ the flag enabled and disabled, without having to reload constants due to load-time feature flag evaluation