Replication Controller returns a 422

[mikejritter]

From #149, this adds a check to make sure we don't add a duplicate entry to a nodes replicating_nodes field. When doing so the uniq constraint on the db table for the (node_id, bag_id) pair is violated, which causes rails to return a 422 Unprocessable Entity.

The only thing I can think of is if this is the best place for this or not. If the replicating_nodes field is not updated in any other location then I suppose it's fine, otherwise having it be in the bag model itself may be appropriate.

[dazza-codes]

Background note: the SDR-dev node has encountered this bug (noted in #149) when doing a bag transfer. The dpn-sync project does the bag transfer. It attempts to update a replication-transfer twice during the bag transfer. Once when the bag is transferred into the staging path (it updates the fixity_value and checks that the from_node has set the store_requested: true value). It then attempts to transfer the bag to a storage facility and update the stored: true value on the from_node. The 422 error was encountered during this second update.

[dazza-codes]

One of the travis builds (for MySQL) has failed, i.e.

Failures:
  1) DataBag has a factory that honors updated_at

     Failure/Error: expect(record.updated_at.change(usec: 0)).to eql time.change(usec: 0)

       expected: 2016-01-06 18:48:50.000000000 +0000
            got: 2016-01-06 18:48:51.000000000 +0000
       (compared using eql?)

       Diff:
       @@ -1,2 +1,2 @@
       -Wed, 06 Jan 2016 18:48:50 UTC +00:00
       +Wed, 06 Jan 2016 18:48:51 UTC +00:00
     # ./spec/models/data_bag_spec.rb:17:in `block (2 levels) in <top (required)>'

This could be a symptom of a fragile rspec matcher. It maybe better to use a be_within matcher that is not too picky about microseconds? e.g. https://www.relishapp.com/rspec/rspec-expectations/v/3-5/docs/built-in-matchers/be-within-matcher

[mikejritter]

Looking at the actual test it shows:

  it "has a factory that honors updated_at" do
    time = 1.year.ago
    record = Fabricate(:data_bag, updated_at: 1.year.ago)
    expect(record.updated_at.change(usec: 0)).to eql time.change(usec: 0)
  end 

While we should probably update this test to use time as an argument in the Fabricate, I don't think this PR is the place for that.

[dazza-codes]

It's OK to make a change in this PR to the spec, e.g. try

time = 1.year.ago
record = Fabricate(:data_bag, updated_at: time) # use time here?
expect(record.updated_at).to be_within(5).of(time.to_i) # or something like that?