TheProlog / prolog-use_cases

Use-case layer for Meldd/Prolog application.
0 stars 0 forks source link

Update 0.14.0 to use current Gems in a reproducible build/test process #77

Closed jdickey closed 7 years ago

jdickey commented 7 years ago

Labelled as a bug not due to known defects in the software itself, but grave defects in the process, or lack thereof, used to build it.

Version 0.14.0 of this Gem preserved dependence on an obsolete, loudly deprecated version of third-party data-structure classes. As a feature of our revision Gem tactic combined with the single massive-ball-of-mud Gem collection installed on the development system, with no continuous-integration system "keeping us honest", we depended on multiple, incompatible versions of third-party Gems. Building such a house of cards on such a ball of mud is simply daring the first gentle breeze to come along and blow the mess down, and a passing typhoon in the form of a reinstalled-from-scratch Ruby environment obliged.

Had any one of these things occurred, the last week of frenzied, overtime-intensive effort would not have. Each and all of them would have been required in any legitimately professional development organisation:

Calling yourself a "startup" must not mean you get to throw the BoK out the window, hunker down underneath the steering wheel, and press the accelerator pedal to the firewall — unless you want to run your "supercar" into a retaining wall at speed. Most of us would rather get where we're going, thanks.

jdickey commented 7 years ago

Now we understand all the "unexpected keys" Dry::Struct::Error messages we've been getting.

Back when we used Virtus for managing attribute collections, we could be sloppy about initialising objects with attribute hashes: as long as everything needed was in there, Virtus' whitelisting would silently ignore any extra rubbish keys that got passed in.

The dry-rb attribute-collection classes, Dry::Struct and Dry::Struct::Value, don't work the same way. If you pass a key/value pair in that has not been defined for the Struct you're instantiating, you'll get a Dry::Struct::Error calling out those unexpected keys. Dry::Types, in order to be "roughly 10-12x faster than Virtus", has to do things quite differently, and one of those differences is that you are responsible for passing in exactly and only the attributes you've specified for a Struct (or Value) instance.

And there's a lot of "sloppy". 😞

jdickey commented 7 years ago

The logical place to fix that sloppiness is to apply a filter based explicitly on the attributes reported for an attribute-container class.

Consider the current Prolog::UseCases::ProposeEditContribution#updated_contribution method and the contribution-entity factory class passed into it by the test code.

The current factory class reads:

  let(:contribution_factory) do
    Class.new do
      attr_reader :created_data

      def initialize
        @created_data = []
        self
      end

      def call(**params)
        # full_params = { identifier: params[:contribution_id] }
        # full_params = full_params.merge params
        obj = Prolog::Entities::Contribution::Proposed.new params
        @created_data << obj
        obj
      end
    end.new
  end

No filtering of the params passed into the entity class instantiation is performed.

This would be an improvement:

  let(:contribution_factory) do
    Class.new do
      ENTITY_CLASS = Prolog::Entities::Contribution::Proposed

      attr_reader :created_data

      def initialize
        @created_data = []
        self
      end

      def call(**params)
        obj = ENTITY_CLASS.new filtered(params)
        @created_data << obj
        obj
      end

      private

      def filtered(params)
        params.select { |attrib, _| ENTITY_CLASS.schema.keys.include? attrib }
      end
    end.new
  end

We guarantee that only valid attribute keys are passed in, because the #filtered method asks the Dry::Struct-derived class for its schema. When running the suitably updated test file, that change eliminates the error previously reported.