MONGOID-5892 - Treat serialize option only with values of nil and [] differently

[bpardee]

When calling serializable_hash(only: []) should suppress all fields. Currently it's treated the same as nil where it gets ignored.

Summary

Previously, passing only: [] to serializable_hash was treated the same as passing only: nil, meaning all fields were included. The correct behavior is to treat an empty array as "include no fields," suppressing all output.

This fix is gated behind the Mongoid.serializable_hash_with_legacy_only feature flag. In 9.1 and later, the flag defaults to true (legacy behavior); set it to false to opt into the new behavior. The default will flip to false in Mongoid 10.0, and the flag will be removed in 11.0.

[Copilot]

Pull Request Overview

This PR fixes a bug in the serializable_hash method where passing an empty array to the :only option was being treated the same as nil, causing all fields to be serialized instead of suppressing all fields as expected.

• Updates the condition to check for nil explicitly rather than checking if the wrapped array is empty
• Adds a test case to verify that serializable_hash(only: []) returns an empty hash

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
lib/mongoid/serializable.rb	Fixes the condition to distinguish between nil and empty array for the :only option
spec/mongoid/serializable_spec.rb	Adds test coverage for the empty array case

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[jamis]

@bpardee -- thank you so much for this PR, we really appreciate the time you've taken to dig into Mongoid and contribute a fix.

Because this PR fixes behavior that has been in place for so long (since 2012!), we're going to play it cautious, here, and move forward with the assumption that changing this behavior may potentially break existing applications.

Our procedure in this case is:

1. Introduce a feature flag (e.g. Mongoid::Config.serializable_hash_with_legacy_only) in the next minor version (i.e. 9.1) that allows the new behavior to be opted into. The flag should default to true when first introduced (requiring the new behavior to be explicitly opted into.)
2. In the next major version (i.e. 10.0), the default for that flag is changed to false, so that this becomes the default behavior, but people can still opt into the old behavior if they need to. (This makes it easier for people to upgrade without fear of their app breaking.) We also deprecate the flag in this version.
3. In the next major version after that (i.e. 11.0), we remove the flag and the legacy behavior.

We've created a ticket in Jira to track this (https://jira.mongodb.org/browse/MONGOID-5892). If you are willing to add the feature flag and corresponding specs to this PR (see Mongoid::Config for examples of other flags), I'd be happy to merge it. I know that's more than you signed up for, though, so if you don't have the time, we'll get to it as part of finalizing the eventual 9.1.0 release.

Thanks again!

[bpardee]

Ok, I'll make the changes but it seems like the values should be switched, i.e., serializable_hash_with_legacy_only == true should be the default and refer to the old behavior?

[johnnyshields]

Great catch @bpardee.

[jamis]

@bpardee yes, you're correct, of course. I was typing faster than I was thinking. :) I've updated my comment to reflect the desired behavior. Thank you for updating the PR! I'll take a look at it today.

[jamis]

I think the logic here is reversed (probably because I misspoke when I first described the requirements).

When serializable_hash_with_legacy_only is true, this condition will be true when a non-nil value was given to :only. Thus, empty array or not, the names list will be intersected with the (possibly empty) array.

When serializable_hash_with_legacy_only is false, this condition will only be true if a non-nil, non-empty value was given to :only. Thus, only non-empty arrays will be intersected with the (possibly empty) array.

The current behavior (and the desired default behavior) is the latter case -- returning everything even if an empty array is given.

[bpardee]

@jamis, I fixed the logic error. Let me know if there is anything else I need to do.

[jamis]

Thank you! This looks great.