[JAVA-2517] Improve documentation for deprecated GridFsFile methods Created: 18/May/17 Updated: 27/Oct/23 Resolved: 18/May/17 |
|
| Status: | Closed |
| Project: | Java Driver |
| Component/s: | GridFS |
| Affects Version/s: | 3.4.2 |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Oliver Gierke | Assignee: | Ross Lawley |
| Resolution: | Works as Designed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Description |
|
GridFsFile.getContentType() is deprecated with a hint to use the metadata document instead. however there's no documentation which field is to be accessed to obtain the content type. Also, it looks like the method is deprecated but doesn't return the content type anymore. What's the reason for the deprecation in the first place. What's wrong with looking up the value from the new location falling back on the old one if none could be found. With the deprecation clients now all of a sudden have to have intimate knowledge about the structure of the metadata document instead of just being able to access the content type, no matter where it's coming from. That's a huge step backwards, isn't it? |
| Comments |
| Comment by Oliver Gierke [ 18/May/17 ] |
|
My point was: from an abstraction level point of view getContentType() was way better as it expresses what I want, not where it's actually obtained from. Now all of a sudden I need to put stuff into a Map and pull it out of it again. It's an implementation detail now leaking into client code I.e. the abstraction level of GridFsFile was lowered, which is completely the opposite of general design best practices. I guess it is what it is now and we're going to just populate a metadata field. Thanks for elaborating though! |
| Comment by Githook User [ 18/May/17 ] |
|
Author: {u'username': u'rozza', u'name': u'Ross Lawley', u'email': u'ross.lawley@gmail.com'}Message: Doc: clarify deprecated top level metadata should be stored in the metadata document
|
| Comment by Ross Lawley [ 18/May/17 ] |
|
Hi oliver.gierke, Apologies for the confusion caused. In 3.1 we added a new implementation of GridFS following the cross driver GridFS specification for GridFS. Prior to this specification GridFS was a convention based upon the driver and there were differences between implementations. The new API specified that all metadata should be stored inside a sub document rather than be top level fields. However, any existing top level fields from existing GridFS data would still accessible via the new API.
The argument for a separate metadata document is it's ultimately more flexible than having named specialised fields. However, we would have legacy users and data migrating to this API and they would need access to the data, hence the inclusion of deprecated methods in a new API.
Looking at the storage code in SpringData you are using the metadata directly when storing the content type: GridFsTemplate.java#L161. I think thats a good idea and worth doing for SpringData to fallback if the metadata.type field does not exist you can lookup the contentType. It depends on your strategy for handling legacy metadata. However, as you are using alternative field names the strategy wouldn't work if it was implemented in the mongo java driver. Hope that answers your questions, I'll look to clarify the API documentation for the 3.5 release. Ross |