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

Plans to support google news sitemaps etc.? #138

Open
corneliuskopp opened this issue Mar 25, 2013 · 11 comments
Open

Plans to support google news sitemaps etc.? #138

corneliuskopp opened this issue Mar 25, 2013 · 11 comments

Comments

@corneliuskopp
Copy link

Hello :)

Are there any plans to support "nonstandard" sitemaps in this project, or open extension points to add more information to sitemaps?

Cheers!
Cornelius

@NightOwl888
Copy link
Collaborator

I have been working on v4 and have created a new interface ISiteMapBuilder that can be implemented to pull sitemap data from any source and use any custom logic to build the sitemap. It also fully supports dependency injection and was designed specifically to handle multiple tenants, so it is highly customizable.

Have a look, and let me know if this meets your requirements.

https://github.com/NightOwl888/MvcSiteMapProvider/tree/v4

@NightOwl888
Copy link
Collaborator

Well, for now I will mark this issue closed. Let me know if you had some other extension point in mind than what is now in the box in v4.

The v4 branch is now part of this repo here: https://github.com/maartenba/MvcSiteMapProvider/tree/v4.

@corneliuskopp
Copy link
Author

Hello NightOwl,

I am terribly sorry for not getting back to you. I got suddenly pulled to a different project and once the flurry was over, I admittedly forgor about this issue -- until now when I returned to what I was initially doing.
The V4 branch looks really promising!

However, it seems I need a bit more than "just" the ISiteMapBuilder as collector, as google's news and multimedia sitemaps expect different contents. So I assume I would have to create a whole custom architecture around

  • the Builder,
  • DynamicNodeProviders for retrieving data,
  • a custom SiteMapNode to contain the extra information and
  • a IXmlSiteMapResultFactory that is able to emit the different kinds of sitemap XML.

Am I right in that assumption?

I promise I will be in touch a lot quicker this time. :)

@NightOwl888
Copy link
Collaborator

@corneliuskopp

Apparently I didn't understand exactly what you were looking for. The term "Site Map" is overloaded too many times in this project and it sometimes becomes difficult to understand what is being referred to.

Just so we're on the same page, I think I now understand that you want to build a search engine site maps resource with specialized content for Google.

After reviewing the XML schemas, it seems that it would require extra objects that currently don't exist to apply this information to the sitemaps xml. For example, a single URL could contain 0 to many image sitemaps, 0 to many videos, etc., so it would not be practical to put this information into a single mvcSiteMapNode element in the .sitemap file (assuming you are using one).

To answer your question, yes you would need to build this as it currently doesn't exist. However, you wouldn't need any DyamicNodeProviders as ISiteMapBuilder is intended to replace them completely in advanced scenarios such as this. You are correct that you would need to add the extra collections for images, videos, and news to a custom ISiteMapNode (or better yet, make the modifications and submit them as a contribution). You are also correct in that you would need to modify or create a new IXmlSiteMapResultFactory to instantiate a custom XmlSiteMapResult with the extra data.

Also, some extra thought needs to go into the XML schema (Mvc.sitemap) if you intend to configure this information in XML rather than in code or in another type of datasource. ISiteMapBuilder gives you the freedom to put this information whereever it is practical, you don't necessarily have to configure it in XML anymore.

Just so this doesn't fall off of the radar, I am reopening this issue.

@NightOwl888 NightOwl888 reopened this May 28, 2013
@corneliuskopp
Copy link
Author

Hi!

I am sorry for the confusion caused in my initial post; yes, you are right, the term is somewhat overloaded! And indeed, I need to create specialized sitemap XML files for search engines (ok, just google p.p), as per the link you included.

Thank you for bringing me up to speed regarding the changes in the node providers and the general course of action.

As the sitemap in my case will be driven by a database, we do not plan to use Mvc.sitemap files; but if my boss allows, I will try to include support nonetheless. Especially if I can negotiate contribution to this project.

However, as I cannot make any promises as to when the implementation will be "ready enough" on our end, this issue might be open for a while. Is that OK?

@NightOwl888
Copy link
Collaborator

Well, one thing is for sure, the job of adding the functionality is quite a bit simpler if you don't use a .sitemap file. Adding the collections to the ISiteMapNode and adding the output to the IXmlSiteMapResult are really all that are needed - then it can be left up to a custom builder that exists outside of this project to populate the data. Building support into the XmlSiteMapBuider could potentially be done later.

Do keep in mind it might also make sense to configure the URLs for images, videos, and news with route values in some cases the same way that it is done on ISiteMapNode - sounds like there is an opportunity to make another shared service. By that I mean encapsulating the logic for route values and URL resolution into a service of its own.

We could just make this a late addition to v4 after it is considered stable - I don't see an issue there.

FYI - I will be making some modifications to the ISiteMapNodeFactory this week to fix #154, which will include a new version of #134. I plan on moving the inheritance functionality into a service that is called by the ISiteMapNodeFactory so it will be consistent across all builders including DynamicNode builders. This might affect your implementation a bit.

@corneliuskopp
Copy link
Author

I had some time to look into the code and plot a course of action for implementation.

I aggree that URL configuration makes sense; I see that a lot of thought and care went into this project, seeing the various existing types and their concerns. I would not want to re-implement existing functionality, so ideally I plan to extend, rather than re-implement the SiteMapNode and the SiteMapResult, e.g. by extracting code into a virtual AddAttributesToUrlNode() method on the result.

Is there, per chance, a form of testing project that exercises the existing features to show how they relate?

@NightOwl888
Copy link
Collaborator

To this point I have only been testing with the included MvcMusicStore project, but building a unit test project is on my wish list. After thinking it through, it makes much more sense to add a test project to fix #154 because it will require testing several scenarios that would be time consuming to do in a full fledged MVC project. I will be starting on the fix today.

@NightOwl888
Copy link
Collaborator

FYI - I have fixed #154, but I ended up not changing the node inheritance logic because it wasn't required for that particular issue. There is another issue #134 that deals with that, but I don't plan on working on that one right now as it is not something I will need.

Anyway, you should be able to work on this now if you haven't started already. If you don't get a chance to contribute your changes, I would appreciate if you could add them to a fork where I can review them so I can add them myself.

@corneliuskopp corneliuskopp mentioned this issue Jun 3, 2013
Closed
@corneliuskopp
Copy link
Author

Thanks for the info! I indeed already started working on this and have my boss' permission to contribute back to the project. I created a pull request for this extensible nodes and sitemap i wrote.

@NightOwl888
Copy link
Collaborator

FYI - I have opened a new issue #345 to gather requirements to address this issue. Your feedback is appreciated.

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

No branches or pull requests

2 participants