[bug] evolve is weird

[midnight-wonderer]

I have a simple model with custom field types here:

class BuggyModel
  include ::Mongoid::Document
  store_in collection: 'buggy_models'

  class HexBytes
    class << self
      def mongoize(hex_string)
        return nil unless hex_string.present?
        [hex_string].pack('H*').then do |bin_content|
          ::BSON::Binary.new(bin_content)
        end
      end

      def demongoize(binary)
        return nil unless binary.present?
        binary.data.then do |bin_content|
          return nil unless bin_content.present?
          bin_content.unpack1('H*')
        end
      end

      # this is weird
      def evolve(hex_string)
        mongoize(hex_string)
      end
    end
  end

  field :_id, type: HexBytes
end

With the declaration, this works:

BuggyModel.create!({ _id: '01' })

but these failed:

BuggyModel.find('01')
BuggyModel.where({ _id: '04' }).first_or_create!

So I remove evolve:

class BuggyModel
  include ::Mongoid::Document
  store_in collection: 'buggy_models'

  class HexBytes
    class << self
      def mongoize(hex_string)
        return nil unless hex_string.present?
        [hex_string].pack('H*').then do |bin_content|
          ::BSON::Binary.new(bin_content)
        end
      end

      def demongoize(binary)
        return nil unless binary.present?
        binary.data.then do |bin_content|
          return nil unless bin_content.present?
          bin_content.unpack1('H*')
        end
      end
    end
  end

  field :_id, type: HexBytes
end

Now the test cases above work, but:

# This failed to fetch the record
BuggyModel.where({ _id: '01' }).first
# which render the following useless
BuggyModel.where({ _id: '04' }).first_or_create!

[midnight-wonderer]

To make it more approachable as test cases, I expect

BuggyModel.create!({ _id: '01' })
BuggyModel.where({ _id: '01' }).first_or_create!
BuggyModel.where({ _id: '02' }).first_or_create!

to create exactly two documents without raising an exception. (on an originally-blank collection)
And all the following

BuggyModel.find('01')
BuggyModel.where({ _id: '01' }).first
BuggyModel.where({ _id: '02' }).first

to fetch Document#01, Document#01, and Document#02 respectively successfully after the steps above.

I can't seem to accomplish that with any configurations I can come up with.

P.S. If you are curious, what are the real-world use cases: I use certificate thumbprints as _id in a collection that stores digital certificates.

[jamis]

Hey @midnight-wonderer -- this does sound like a bug! I've added an issue for it on our tracker (here: https://jira.mongodb.org/browse/MONGOID-5633) and we'll discuss it as a team on Monday. I can't make any promises that we'll get to it right away, but you can follow that issue if you'd like.

[jamis]

Okay, I looked into it, and I think the bug might actually be in your mongoize/demongoize logic. (And in Mongoid's documentation!)

First of all, your #evolve method is correct -- having it call mongoize is correct in this case. However, the thing to note here is that #mongoize might be called on a value that is already mongoized. This can happen in create, for instance, since the criterion's selector is already mongoized, and is used verbatim by the create. When the new document is instantiated, the attributes are mongoized again.

Thus, you need to make sure your mongoize method can correctly identify and handle values that are already of the required type.

Secondly (and this is just something I noticed as I was investigating this--I don't think it relates directly to the issues you noticed): in your demongoize method, you are using String#present? to detect whether the bin_content is present. However, String#present? may return false even if the string has a non-zero length! A string containing only spaces, for instance, is considered "not present":

"   ".present? #-> false

When considering binary data, you probably just want to compare the value to nil directly, or (more concisely) use it as a truthy value:

return nil if bin_content.nil?
# or
return nil unless bin_content

When I made those changes, your code ran correctly, including the expectations mentioned in your comment. Can you verify? If I've misunderstood, please do clarify and I'll look again.

[midnight-wonderer]

OK seems to work on my end too.

class HexBytes
  class << self
    def mongoize(hex_string)
      # idempotent handling
      return hex_string if hex_string.is_a?(::BSON::Binary)
      return nil unless hex_string.present?
      [hex_string].pack('H*').then do |bin_content|
        ::BSON::Binary.new(bin_content)
      end
    end

    def demongoize(binary)
      # idempotent handling
      return binary if binary.is_a?(::String)
      return nil unless binary.present?
      binary.data.then do |bin_content|
        return nil unless bin_content.present?
        bin_content.unpack1('H*')
      end
    end

    def evolve(hex_string)
      mongoize(hex_string)
    end
  end
end

Thanks for the investigation and the recommendation regarding nil.

From my memory, I don't recall the document mentioning anything regarding idempotency, but some example includes something I don't understand, so I did not implement similar functionalities to make the oddity noticeable when something went wrong.

Thanks a lot; you may close this and probably update the document, then.

P.S. In the application I am working on, the string is supposed to be a hex string; if not, someone is probably doing the integration wrong.