[JAVA-3331] InsertOneOptions and InsertManyOptions are final Created: 19/Jun/19 Updated: 16/Dec/20 Resolved: 15/Dec/20 |
|
| Status: | Closed |
| Project: | Java Driver |
| Component/s: | Query Operations |
| Affects Version/s: | 3.10.2 |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Justin Lee | Assignee: | Unassigned |
| Resolution: | Won't Fix | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Description |
|
These options class, unlike most/all of the others, are final. This makes it awkward for me to provide a consistent experience in Morphia. With the others, I can extend them and add things like WriteConcern and ReadPreference options to provide a single place for users to define various options. With these being final, i have to jump through certain hoops and it results in an inconsistent API and implementations. 'final' should be removed from these types. I can work around it but it makes me sad. |
| Comments |
| Comment by Jeffrey Yemin [ 16/Dec/20 ] |
|
All the options classes ideally would be final but doing so would break binary compatibility, so I'm not inclined to make the change at this time. We just have to live with it. In practice, it's not likely that ordinary users would ever even think to extend any of these classes, so in practice it's not a big issue. |
| Comment by Heng Wang [ 16/Dec/20 ] |
|
hi. jeffrey yemin ,I create the PR for this issues. I found that the options are not uniform, not all of them were made final. such as updateOptions . I think they should be a unified standard. Please let me know your research direction . |
| Comment by Jeffrey Yemin [ 15/Dec/20 ] |
|
OK, thanks justin.lee. |
| Comment by Justin Lee [ 15/Dec/20 ] |
|
This is what I ended up doing and, honestly, it's probably a better choice. I added write concern and read preferences to the options, locally. That was my motivation and it's probably best to make a clean distinction between Morphia's and the driver's options. I was going for "use either depending on your needs" but perhaps it's just best to funnel everyone through the Morphia APIs. I might even generate those classes to better track the changes ( like `allowDiskUse` that just showed up in `FindOptions` ). Let's go ahead and close this as "won't fix" or "invalid" (whatever Jira calls it these days). |
| Comment by Jeffrey Yemin [ 15/Dec/20 ] |
|
Just got a PR for this, but probably best to discuss the merits here instead of there. If an application were to subclass either of those classes and pass an instance of the subclass to the driver, the driver would not be aware of the subclass and so would ignore any additional fields from the subclass. That's why the options classes should final, to indicate that they are not designed for extension. It was a mistake that not all of them were made final. For a wrapping driver like Morphia, I think it's best to wrap options rather than extend them. It's a bit more work for Morphia, but the benefit is a clearer contract for everyday driver users. |