Merge pull request #113 from alphagov/iterate-random-example

Avoid double validation of payloads

CHANGELOG.md

@@ -1,3 +1,7 @@
+# Unreleased
+
+* Improve speed of random schema generation for customised content items
+
 # 5.0.0
 
 * BREAKING: Drop support for Ruby 3.0. The minimum required Ruby version is now 3.1.4.

lib/govuk_schemas/random_example.rb

@@ -29,7 +29,8 @@ class RandomExample
     #     GovukSchemas::RandomExample.new(schema: schema, seed: 777).payload
     #     GovukSchemas::RandomExample.new(schema: schema, seed: 777).payload # returns same as above
     #
-    # @param [Hash] schema A JSON schema.
+    # @param [Hash]         schema  A JSON schema.
+    # @param [Integer, nil] seed    A random number seed for deterministic results
     # @return [GovukSchemas::RandomExample]
     def initialize(schema:, seed: nil)
       @schema = schema
@@ -54,8 +55,6 @@ def initialize(schema:, seed: nil)
     # @param [Block] the base payload is passed inton the block, with the block result then becoming
     #   the new payload. The new payload is then validated. (optional)
     # @return [GovukSchemas::RandomExample]
-    # @param [Block] the base payload is passed inton the block, with the block result then becoming
-    #   the new payload. The new payload is then validated. (optional)
     def self.for_schema(schema_key_value, &block)
       schema = GovukSchemas::Schema.find(schema_key_value)
       GovukSchemas::RandomExample.new(schema:).payload(&block)
@@ -80,24 +79,40 @@ def self.for_schema(schema_key_value, &block)
     #   the new payload. The new payload is then validated. (optional)
     # @return [Hash] A content item
     # @raise [GovukSchemas::InvalidContentGenerated]
-    def payload
+    def payload(&block)
       payload = @random_generator.payload
-      # ensure the base payload is valid
+
+      return customise_payload(payload, &block) if block
+
       errors = validation_errors_for(payload)
       raise InvalidContentGenerated, error_message(payload, errors) if errors.any?
 
-      if block_given?
-        payload = yield(payload)
-        # check the payload again after customisation
-        errors = validation_errors_for(payload)
-        raise InvalidContentGenerated, error_message(payload, errors, customised: true) if errors.any?
-      end
-
       payload
     end
 
   private
 
+    def customise_payload(payload)
+      original_payload = payload
+      customised_payload = yield(payload)
+      customised_errors = validation_errors_for(customised_payload)
+
+      if customised_errors.any?
+        # Check if the original payload had errors and report those over
+        # any from customisation. This is not done prior to generating the
+        # customised payload because validation is time expensive and we
+        # want to avoid it if possible.
+        original_errors = validation_errors_for(original_payload)
+        errors = original_errors.any? ? original_errors : customised_errors
+        payload = original_errors.any? ? original_payload : customised_payload
+        message = error_message(payload, errors, customised: original_errors.empty?)
+
+        raise InvalidContentGenerated, message
+      end
+
+      customised_payload
+    end
+
     def validation_errors_for(item)
       JSON::Validator.fully_validate(@schema, item, errors_as_objects: true)
     end

spec/lib/random_example_spec.rb

@@ -32,34 +32,44 @@
       expect(first_payload).to eql(second_payload)
     end
 
-    it "can customise the payload" do
-      schema = GovukSchemas::Schema.random_schema(schema_type: "frontend")
-
-      example = GovukSchemas::RandomExample.new(schema:).payload do |hash|
-        hash.merge("base_path" => "/some-base-path")
-      end
+    it "raises an error if the generated schema is invalid" do
+      generator = instance_double(GovukSchemas::RandomSchemaGenerator, payload: {})
+      allow(GovukSchemas::RandomSchemaGenerator).to receive(:new).and_return(generator)
 
-      expect(example["base_path"]).to eql("/some-base-path")
+      schema = GovukSchemas::Schema.random_schema(schema_type: "frontend")
+      expect { GovukSchemas::RandomExample.new(schema:).payload }
+        .to raise_error(GovukSchemas::InvalidContentGenerated)
     end
 
-    it "failes when attempting to edit the hash in place" do
-      schema = GovukSchemas::Schema.random_schema(schema_type: "frontend")
+    context "when the payload is customised" do
+      it "returns the customised payload" do
+        schema = GovukSchemas::Schema.random_schema(schema_type: "frontend")
 
-      expect {
-        GovukSchemas::RandomExample.new(schema:).payload do |hash|
-          hash["base_path"] = "/some-base-path"
+        example = GovukSchemas::RandomExample.new(schema:).payload do |hash|
+          hash.merge("base_path" => "/some-base-path")
         end
-      }.to raise_error(GovukSchemas::InvalidContentGenerated)
-    end
 
-    it "raises if the resulting content item won't be valid" do
-      schema = GovukSchemas::Schema.random_schema(schema_type: "frontend")
+        expect(example["base_path"]).to eql("/some-base-path")
+      end
 
-      expect {
-        GovukSchemas::RandomExample.new(schema:).payload do |hash|
-          hash.merge("base_path" => nil)
-        end
-      }.to raise_error(GovukSchemas::InvalidContentGenerated)
+      it "raises if the resulting content item won't be valid" do
+        schema = GovukSchemas::Schema.random_schema(schema_type: "frontend")
+
+        expect {
+          GovukSchemas::RandomExample.new(schema:).payload do |hash|
+            hash.merge("base_path" => nil)
+          end
+        }.to raise_error(GovukSchemas::InvalidContentGenerated, /The item was valid before being customised/)
+      end
+
+      it "raises if the non-customised content item was invalid" do
+        generator = instance_double(GovukSchemas::RandomSchemaGenerator, payload: {})
+        allow(GovukSchemas::RandomSchemaGenerator).to receive(:new).and_return(generator)
+
+        schema = GovukSchemas::Schema.random_schema(schema_type: "frontend")
+        expect { GovukSchemas::RandomExample.new(schema:).payload { |h| h.merge("base_path" => "/test") } }
+          .to raise_error(GovukSchemas::InvalidContentGenerated, /An invalid content item was generated/)
+      end
     end
   end
 end