sitemap job refactoring to improve extensibility

[Remo]

I've had some issues when I wanted to override the sitemap job.
All I wanted to do is to add some custom entries without overriding the whole file.

Unfortunately most of the logic was in run(), a method I didn't want to override.

I created a new method "AddPages" which you can override. At this point you can easily add additional entries to the xml file, no need to override the whole run method or remove first

[mlocati]

@Remo are you thinking about the GenerateMultilingualSitemap job of the multilingual package, aren't you?
I also would like to optimize the somehow duplicated functions of the two jobs generating sitemap.xml (GenerateSitemap and GenerateMultilingualSitemap).
What about moving the AddPages somehow to the Page or PageList models and adding an event for each generated xml node? In this way the multilingual package could simply intercept this event and doing its stuff within this event...

[Remo]

@mlocati No, it's not about GenerateMultilingualSitemap. We've got quite a few projects with page type controllers. There are methods we call and get different content based on the parameters. /page/object-123 while object-123 is a database object and not a page object. I'd like to make sure Google finds those as well and wanted to override.

Nothing else than additional pages I'd like to add to sitemap.xml

However, I do see that GenerateMultilingualSitemap might benefit from a refactoring. I'm not sure if that method fits well into Page or PageList, generating sitemap.xml seems to be pretty specific to this job.

I also thought about adding two new events
on_sitemap_xml
on_page_sitemap_xml

The first would get called once for every job run and the second one would get called for every page found in the job.

I'll keep on thinking about that, but so far, the "AddPages" method is all I need.

[mlocati]

@Remo Ok. BTW we could also leave the AddPage inside the GenerateSitemap job, add those two events.
This way the GenerateMultilingualSitemap job of the multilingual package is useless: multilingual could simply doing its stuff byintercepting the events (and there's less problems in having two jobs generating sitemap.xml).

As a side note, I can't remember why I kept two separated calls to AddPage for the home and the other pages: now that you're touching the GenerateSitemap job, could you remove the self::AddPage($hFile, 1, $instances) line and the WHERE (cID > 1) sql condition?

[Remo]

@mlocati cID > 1 stuff removed

[mlocati]

Useless stuff removed almost everywhere ;)

[Remo]

thanks @mlocati

[Remo]

The changes made by @mlocati add two events as described above and also introduce SimpleXML. The memory overhead is a bit bigger but the code is much cleaner and easier to extend, more about that here Remo#2

[aembler]

Accidentally closed this earlier.

[aembler]

Thanks guys this looks good.

[mlocati]

See also concretecms/addon_internationalization#45