search

thecadams / 301RedirectModule

An improved version of the Sitecore 301 redirect module by Chris Castle, available at http://trac.sitecore.net/301RedirectModule
MIT License
30 stars 67 forks source link

Fix performance issues by changing from SelectSingleItem to GetItem #75

Closed markgibbons25 closed 8 months ago

markgibbons25 commented 8 months ago

Fix performance issues by changing from SelectSingleItem to GetItem so that it is cached and doesn't hit the DB every time.

Also remove fast query which is a known performance issue waiting to happen and deprecated feature.

thecadams commented 8 months ago

Hi, thanks for the PR! I’m not familiar with Sitecore these days but very happy to merge based on the way it looks.

Any concerns to be aware of for other users of the code with this PR in it?

markgibbons25 commented 8 months ago

Thanks @thecadams , no concerns that I'm aware of, it should be backwards compatible to all supported versions. I've tested on Sitecore 10.3.

thecadams commented 8 months ago

Thanks. Going to merge on the back of what you’re saying!

Incidentally I’ve been unable to maintain this module lately and am looking for a new maintainer. If you’re interested, let’s chat!

thecadams commented 8 months ago

Congrats on becoming one of our contributors by the way. Going to have to add your name to the credits! 😃

steviemcg commented 8 months ago

Bit late to comment after the merge, but had some feedback.

Users choosing fast will now fallback to .GetItem and probably crash. I'd fallback to query and add something to the logs.

SelectSingleItem supported query syntax and the new code doesn't. Why not check whether it's a query and decide which method to use accordingly.

The performance benefits in this PR are obvious but they're less backwards compatible than I'd hope for in a public library.

thecadams commented 8 months ago

Hey there @steviemcg sounds like I merged a little too quick, but it’s all good, what if we reinstate the fast path?