Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix id duplication #913

Closed
wants to merge 1 commit into from
Closed

Fix id duplication #913

wants to merge 1 commit into from

Conversation

abhiarya-uvic
Copy link

We are students from University of Victoria working on architectural review of Strapi system as part of our course project. We found that the id and _id are duplicated in the JSON response from the API. We also analysed previous issues mentioned and we know that Strapi is handling two ORM right now (Mongoose and Bookshelf) and that's why it is duplicated.

However, it is unnecessary to return redundant value of ids (id and _id) with every API request made. So, we are proposing changes to Strapi-generate-api package to bandaid this issue.

@Aurelsicoko
Copy link
Member

@abhiarya-uvic Thank you (or all of you) for this PR! You're right it's unnecessary to return identicals values in the same object. In this case, I think we need to handle the issue deeper.

  • First of all, the issue is mainly caused by MongoDB. The _id attribute is specific to MongoDB and most of the others databases (SQL) are using the id attribute as a unique identifier. To me, it makes more sense to use the id attribute everywhere to keep our codebase consistent.

  • So, I would focus my effort on MongoDB which means in our connector strapi-mongoose. And I would try to hide the _id attribute and replace it with the id attribute.

  • I also need to make sure the id attribute is never saved. Currently, it's only the case when you're using the Content Manager (administration panel). If you're using the API to create the entries, the issue doesn't appear.


Here my proposal

collection.schema.pre('save', function (next) {
  // Delete the id and avoid to save it.
  delete this.id;
  
  next();
});

collection.schema.options.toObject = collection.schema.options.toJSON = {
  virtuals: true,
  transform: function (doc, returned, opts) {
    returned.id = returned._id;

    delete returned._id;
  }
};

This solution virtualises the id attribute. It removes it from the database and it always displays the id attribute instead of _id if you're not fetching data with the lean method.

@abhiarya-uvic
Copy link
Author

@Aurelsicoko We had a discussion among our team members and we think that the solution you proposed seems better to rectify this issue. Hiding the _id attribute and replacing it with the id attribute looks good. Thanks so much!

@Aurelsicoko
Copy link
Member

@abhiarya-uvic Awesome, we're waiting for the changes in this file (https://github.com/strapi/strapi/blob/master/packages/strapi-mongoose/lib/index.js). Feel free to ask us anything if you need help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants