[GODRIVER-572] TransformDocument should document how it handles nil Created: 18/Sep/18  Updated: 28/Oct/23  Resolved: 21/Feb/19

Status: Closed
Project: Go Driver
Component/s: Documentation
Affects Version/s: None
Fix Version/s: 1.0.0-rc1

Type: Improvement Priority: Minor - P4
Reporter: David Golden Assignee: Isabella Siu (Inactive)
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Epic Link: GoDocs for 1.0

 Description   

nil is equivalent to an empty document, but this is not documented.



 Comments   
Comment by Githook User [ 21/Feb/19 ]

Author:

{'name': 'Isabella Siu', 'email': 'isabella.siu@10gen.com', 'username': 'iwysiu'}

Message: GODRIVER-572 document that Client, Collection, and Database methods will return an ErrNilDocument if nil is passed in for an interface{}

Change-Id: I1dd743891dd2ca49ebee0b76fa15f5ea738813a4
Branch: master
https://github.com/mongodb/mongo-go-driver/commit/f085d9c2b7466c04f3fd356fe1174c5f614e1632

Comment by Isabella Siu (Inactive) [ 20/Feb/19 ]

code review: https://review.gerrithub.io/c/mongodb/mongo-go-driver/+/445618

Comment by David Golden [ 07/Jan/19 ]

I don't think that's enough. Empty interface is like void*. Just saying "non-nil" isn't enough. We should be positively listing what types/interfaces are allowed.

(Doing it in doc.go seems fine, though.)

Comment by Divjot Arora (Inactive) [ 07/Jan/19 ]

I think adding it to doc.go is good enough. Just a small note like "all methods in collection.go that take params of type interface{} will return ErrNilDocument if nil is passed in"

Comment by David Golden [ 07/Jan/19 ]

As far as I can see, there is zero documentation in the mongo package about what types are acceptable to the empty interface parameters that are intended to take "documents".

I think now that TransformDocument is private, there still needs to be such documentation somewhere. (See for example, this terminology section in Perl driver documentation). So perhaps this just needs to be renamed to "Document what types are allowed as documents for empty interface parameters".

Comment by Kristofer Brandow (Inactive) [ 07/Jan/19 ]

We have ErrNilDocument, but if that's not enough we can call it out in the main documentation in the doc.go file. I don't think we should add it to every single CRUD method.

Comment by Divjot Arora (Inactive) [ 07/Jan/19 ]

yep - not sure if we should make a new ticket for the docs or just modify this ticket to add documentation for all CRUD functions to say when nil is and isn't allowed.

Comment by Kristofer Brandow (Inactive) [ 07/Jan/19 ]

It's transformDocument itself that returns an error if you provide it with nil, so it'll return an error for inserts and finds alike.

Comment by Divjot Arora (Inactive) [ 07/Jan/19 ]

Things like the filter for Find(), however, still pass through transformDocument() without being checked for nil.

Previous comment was incorrect. transformDocument() now checks to see if the value passed in is nil and returns an error if it is. This behavior isn't document anywhere, though. At the very least, the documentation for Find() should say that a nil filter isn't allowed.

Comment by Kristofer Brandow (Inactive) [ 07/Jan/19 ]

Since GODRIVER-659 we no longer allow nil as a valid parameter for anything that goes through transformDocument. Given that, do we still need to document this?

Comment by David Golden [ 11/Oct/18 ]

It's possible there needs to be a top-level documentation section on how empty interface documents are handled across the API (assuming that behavior is universal).

Comment by Divjot Arora (Inactive) [ 11/Oct/18 ]

From what I understand, the TransformDocument function previously used by collection.go has been replaced by a transformDocument function defined in mongo/mongo.go. The documentation for the collection.go functions should be updated to reflect this because many of them say "See TransformDocument for the list of valid types for documents."

I'm not sure how docs for private functions work. Should transformDocument be documented for users or should each collection.go function that uses it specify the types that can be passed to it?

Generated at Thu Feb 08 08:34:30 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.