Skip to content

Commit

Permalink
Fix incorrect error for invalid customised payload
Browse files Browse the repository at this point in the history
The change in #113 didn't
handle a common usage scenario where a user modifies the payload passed
into the block (example usage [1]).

In order to handle this we need to make a copy of the original payload
before it is modified. As this project doesn't have an ActiveSupport
dependency I wrote the equivalent of a deep_dup method. using
`Marshal.load(Marshal.dump(object))`, with an expectation that given the
data generated is JSON in a Ruby Hash/Array structure there'd be no
risk of objects not having Marshal support.

Marshal is a Ruby module that can serialise Ruby objects into a byte
stream and can take a byte stream and re-create the objects. By doing
this for a payload it creates new objects for the entire payload hash.
This means that any modifications to the payload are not represented in
the original_payload object.

An earlier approach was to iterate ourselves, but that was a bit more
verbose:

```
    def deep_dup_payload(item)
      case item
      when Array
        item.map { |i| deep_dup_payload(i) }
      when Hash
        item.dup.transform_values { |v| deep_dup_payload(v) }
      else
        item.dup
      end
    end
```

Anecdotally it does seem that previously this project considered
modifying the payload as unsupported [2], but this wasn't enforced by a
frozen object and would now be a breaking change to require this.

[1]: https://github.com/alphagov/content-data-api/blob/cddc744baa0a073868a527b5513a7943fa53ddc6/spec/factories/messages.rb#L19-L26
[2]: https://github.com/alphagov/govuk_schemas/blob/e1da17587b10df8ae3b29d7583c3a4881a377f97/spec/lib/random_example_spec.rb#L45-L53
  • Loading branch information
kevindew committed May 23, 2024
1 parent 96875f3 commit 834e142
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 1 deletion.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# Unreleased

* Fix issue where customised schema validation message was incorrect for a modified payload

# 5.0.1

* Improve speed of random schema generation for customised content items
Expand Down
3 changes: 2 additions & 1 deletion lib/govuk_schemas/random_example.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ def payload(&block)
private

def customise_payload(payload)
original_payload = payload
# Use Marshal to create a deep dup of the payload so the original can be mutated
original_payload = Marshal.load(Marshal.dump(payload))
customised_payload = yield(payload)
customised_errors = validation_errors_for(customised_payload)

Expand Down
11 changes: 11 additions & 0 deletions spec/lib/random_example_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,17 @@
}.to raise_error(GovukSchemas::InvalidContentGenerated, /The item was valid before being customised/)
end

it "raises when modifying the hash directly creates an invalid content item" do
schema = GovukSchemas::Schema.random_schema(schema_type: "frontend")

expect {
GovukSchemas::RandomExample.new(schema:).payload do |hash|
hash["base_path"] = nil
hash
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)
Expand Down

0 comments on commit 834e142

Please sign in to comment.