Geta / geta-optimizely-sitemaps

Search engine sitemaps.xml for Optimizely CMS 12 and Commerce 14
Apache License 2.0
11 stars 15 forks source link

Sitemaps file not taken from database #59

Open tomasz-jankowski-tj opened 2 years ago

tomasz-jankowski-tj commented 2 years ago

Sitemap file is generated each request instead of using file generated by scheduled job and stored in database.

How it looked for Epi 11

 if (sitemapData.Data == null || (SitemapSettings.Instance.EnableRealtimeSitemap))
            {
                if (!GetSitemapData(sitemapData))
                {
                    Log.Error("Xml sitemap data not found!");
                    return new HttpNotFoundResult();
                }
            }

            CompressionHandler.ChooseSuitableCompression(Request.Headers, Response);

            return new FileContentResult(sitemapData.Data, "text/xml; charset=utf-8");

How it looks in EPI 12:

var sitemapData = _sitemapRepository.GetSitemapData(Request.GetDisplayUrl());

            if (sitemapData == null)
            {
                return SitemapDataNotFound();
            }

            if (_configuration.EnableRealtimeSitemap)
            {
                return RealtimeSitemapData(sitemapData);
            }

            return SitemapData(sitemapData);
marisks commented 2 years ago

Looking at the code and it works the same as before. It is refactored so that it is easier to understand and that's all.

debski commented 2 years ago

SitemapData() does go through all pages and generate a sitemap in real time still, right? Only difference seems to be that RealtimeSitemapData() does not persist to database after generating.

marisks commented 2 years ago

It seems so. RealtimeSitemapData also caches results in memory if caching is enabled. I am not sure why something like this was made, but there is a task to improve this: https://github.com/Geta/geta-optimizely-sitemaps/issues/39 Although, I do not have time for it.

dan-valea commented 2 years ago

Hi,

This issue introduces significate performance issues. The sitemap that was created by the scheduled job is completely ignored and regenerated each time the sitemap is accessed, no matter if the EnableRealtimeSitemap setting is used or not. Here's how it looks now https://www.screencast.com/t/Cs2PLrZX85f vs how it used to be in Opti 11 https://github.com/Geta/SEO.Sitemaps/blob/master/src/Geta.SEO.Sitemaps/Controllers/GetaSitemapController.cs

notice the return new FileContentResult(sitemapData.Data, "text/xml; charset=utf-8"); in Opti 11 which makes use of what was previously generated by the Scheduled Job.

This can be fixed by replacing line 60 here: https://github.com/Geta/geta-optimizely-sitemaps/blob/master/src/Geta.Optimizely.Sitemaps/Controllers/GetaSitemapController.cs with return FileContentResult(sitemapData);

tomaszmadeyski commented 1 year ago

Looks like there's already a PR for this ? #74