Performance: Change `includes` to be an object

[pedantic-git]

Hi Contentful!

In using this gem with include_level set to high-ish numbers on complex content, we find it spends as much as 57% of its time in the coerce_link function, specifically in the call to Support.resource_for_link, which iterates over the includes array in order looking for the linked content.

This PR refactors the whole concept of includes in the gem to be an object of class Contentful::Includes and moves the two related methods from Support into that class. In doing so, we get a massive performance boost from storing a lookup hash inside the object for whenever this method is needed.

The new class implements the Contentful::ArrayLike interface, so it should be backwards compatible with most (all?) custom resource classes people have added on top of the default ones. It isn't backwards compatible with anyone who adds :includes_for_single to their default configuration, but I don't think this is a public interface anyway (I couldn't find it referenced in your docs).

It passes all Rspec tests and works for at least basic tests in our Contentful app. I've tried to follow the Contentful coding style wherever I understood it!

Let us know what you think! Happy to answer any questions about this.

[pedantic-git]

Hi @rubydog - I was wondering if you've had chance to cast your eyes over this at all? We're about to take code into production with this fix included so it would be great to know if we're doing anything terrible from your perspective!

Thank you!!

[rubydog]

Hey @pedantic-git, thanks for the PR, I haven't had chance to look at it yet, I am swamped with some feature requests. Let me check with support team and see if I can prioritise this.

Cheers Bhushan

[pedantic-git]

Thanks for the update! I understand what it's like to be under pressure from every direction! Just wanted to make sure you'd seen it.

[rubydog]

@pedantic-git do you have a deadline until when you use this in production?

[pedantic-git]

@rubydog We actually took it into production today but we have a rollback strategy if it turns out to be problematic. Not seen any issues so far!

[pedantic-git]

Hi @rubydog - just a reminder about this PR! We've been using this fork in production for some months now without any issues (well, we hit one additional performance snag 4 months ago but it's been resolved in the 2 commits above). Have you given any further thought to incorporating this PR or any review comments?

I'd also like to introduce @suzannehamilton who is taking over overall responsibility for our Contentful-based product at Citizens Advice. I will still be working at the org and will be able to respond to comments on this PR but Suzanne is now in charge of the day-to-day.

[rubydog]

@suzannehamilton can you please merge the latest master in your pr branch? Then we can run circleci jobs on the PR and if everything passes we will merge the PR in master.

[suzannehamilton]

@rubydog Thanks for the update! I've just merged the latest master into this branch.

I'm also running our CI build for the app which uses this gem.

[rubydog]

@suzannehamilton one last thing. Could you please also update the changelog as I am unable to commit any changes in your PR.

Thanks!

[suzannehamilton]

@rubydog Sorry for the slow reply - I was on holiday! I've just updated the changelog.

[suzannehamilton]

I've deleted the trailing whitespace that was causing the Rubocop failures.

Let me know if you want me to tidy up the commit history. I didn't want to force-push commits without warning you!

[suzannehamilton]

Thanks very much! 🎉