Skip to content

Commit

Permalink
Avoid double validation of payloads
Browse files Browse the repository at this point in the history
The call to validate a schema is quite expensive:

```
[3] pry(#<GovukSchemas::RandomExample>)> puts Benchmark.measure { 100.times { JSON::Validator.fully_validate(@Schema, item, errors_as_objects: true) } }
  7.614167   1.532485   9.146652 ( 10.468765)
```

This updates the code to avoid making two calls to the validator when a
payload is generated with a block if the customised version is valid.

Before:

```
irb(main):010:0> puts Benchmark.measure { 100.times { GovukSchemas::RandomExample.for_schema(frontend_schema: "news_artic
le") { |schema| schema["title"] = "Custom"; schema } } }
 13.308641   3.087679  16.396320 ( 18.977005)
```

After:

```
irb(main):002:0> puts Benchmark.measure { 100.times { GovukSchemas::RandomExample.for_schema(frontend_schema: "news_artic
le") { |schema| schema["title"] = "Custom"; schema } } }
  7.654303   1.561620   9.215923 ( 10.604802)
```

The motivation for this was that the GOV.UK Chat test suite makes quite
heavy use of random schemas with customisations for checking supported
schemas. On a MBP with GOV.UK Docker this reduces the time of the
running the test suite from around ~55s to around ~35s.
  • Loading branch information
kevindew committed May 22, 2024
1 parent e1da175 commit 3a7e16b
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 30 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
34 changes: 25 additions & 9 deletions lib/govuk_schemas/random_example.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,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
Expand Down
52 changes: 31 additions & 21 deletions spec/lib/random_example_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 3a7e16b

Please sign in to comment.