Added support for building a sitemap using a fluent api.

[pauldotknopf]

No description provided.

[maartenba]

Looks pretty good to me, guess we can probably merge this one in.

[pauldotknopf]

Do have an ETA with this would be merged in and released on NuGet? I have an open source project that depends on this and it will be distributed via NuGet.

[NightOwl888]

Here are a few examples (pseudo code) of how I envision the fluent API will work in the ISiteMapNodeProvider. Don't worry too much about IDynamicNodeProvider at this point - we might have to wait until v5 for support there to avoid breaking backward compatibility. The only thing to keep in mind is that the services used for the fluent API should be portable so they can be used there as well (probably by adding another façade service similar to the ISiteMapNodeHelper).

public class MySemanticSiteMapNodeProvider
    : ISiteMapNodeProvider
{
    public MySemanticSiteMapNodeProvider()
    {
    }
    protected const string SourceName = "MySemanticSiteMapNodeProvider";

    #region ISiteMapNodeProvider Members

    public IEnumerable<ISiteMapNodeToParentRelation> GetSiteMapNodes(ISiteMapNodeHelper helper)
    {
        return helper.Add(this.SourceName)
            .Title("Home")
            .Key("Home") // Specify explicit key so other node providers can attach nodes here
            .Description("This is the home page")
            .Controller("Home")
            .Action("Index")
            .ChangeFrequency(ChangeFrequency.Always)
            .UpdatePriority(UpdatePriority.Normal)
            .ChildNodes(homeChildren =>
                homeChildren.Add() // This add method should inherit the SourceName if not provided explicitly
                    .Key("Genres")
                    .Title("Browse")
                    .Controller("Store")
                    .Action("Index")
            )
            .ToList(); // Calling ToList() converts the structure into an IList<ISiteMapNodeToParentRelation>

        // NOTE: the fluent API (.ToList()) should also support .Union() so we can append 
        // parallel trees of nodes to the first one. This is because the home page won't 
        // necessarily be defined in this SiteMapNodeProvider and it is not very efficient 
        // to use result.AddRange() to append additional lists of nodes from other trees.
    }

    #endregion
}

public class MyDynamicSiteMapNodeProvider
    : ISiteMapNodeProvider
{
    public MyDynamicSiteMapNodeProvider(IMyEntityFactory myEntityFactory)
    {
        if (myEntityFactory == null)
            throw new ArgumentNullException("myEntityFactory");
        this.myEntityFactory = myEntityFactory;
    }
    protected readonly IMyEntityFactory myEntityFactory;
    protected const string SourceName = "MyDynamicSiteMapNodeProvider";

    #region ISiteMapNodeProvider Members

    public IEnumerable<ISiteMapNodeToParentRelation> GetSiteMapNodes(ISiteMapNodeHelper helper)
    {
        using (var db = this.myEntityFactory.GetContext())
        {
            // Create node for each genre
            foreach (var genre in db.Genres)
            {
                yield return helper
                    .Add(this.SourceName)
                    .Key("Genre_" + genre.Name)
                    .ParentKey("Genres")
                    .Title(genre.Name)
                    .RouteValues.Add("genre", genre.Name)
                    .Controller("Store")
                    .Action("Index")
                    .Single(); // Single would return just the one node (ISiteMapNodeToParentRelation) as it would in linq
            }

            // Create node for each album
            foreach (var album in db.Albums.Include("Genre"))
            {
                yield return helper
                    .Add(this.SourceName)
                    .Title(album.Title)
                    .ParentKey("Genre_" + album.Genre.Name)
                    .Key("Album_" + album.AlbumId)
                    .Controller("Store")
                    .Action("Browse")
                    .RouteValues.Add("id", album.AlbumId)
                    .Single();
            }
        }

        // Add a node using the regular syntax
        var node = helper.CreateNode("MyNode", "Home", this.SourceName)

        // Set Values
        node.Title = "MyNode";
        node.Controller = "MyController";
        node.Action = "MyAction";

        yield return node;

    }

    #endregion
}

Keep in mind this isn't set in stone, but just food for thought. There are several things to fix to get it to be this streamlined enough for prime time (for example, the code to automatically add an area as empty string by default should be moved either inside of the SiteMap or into the helper). In fact, this is the reason why ISiteMapNodeProvider isn't documented as an alternative to IDynamicNodeProvider - because there are still many small details (mostly correct default values) that should be handled by the framework which are still currently required of the ISiteMapNodeProvider, which makes building custom implementations more complex than they need to be.

One thing to be aware of is that the MVC framework defaults to "Index" if no action is provided, and so should we. Therefore, leaving off .Action("Index") should be possible (whether or not using the fluent API) to make the declaration more concise.

FYI - I have fixed the source in the dev branch to exclude the "known" attributes as well as controller, action, and area from the Attributes collection.

[NightOwl888]

Haven't heard from you in awhile and I am curious as to your progress on the fluent API.

[NightOwl888]

@theonlylawislove

Just wanted to let you know that I am working on putting together a minor version release, probably within the next few days. Since this change clearly falls into the minor release category, I'd really like to include it in this release, if time permits.

If you are no longer interested in doing this, that is fine, but I would appreciate you letting us know. I think your implementation is very close to what we would like - the interface looks pretty good, it just needs to be wired in better, make use of existing services, and polished up.

[pauldotknopf]

Hey,

Sorry, meant to comment sooner. I was developing this feature as a requirement for a project that I am working on, but I do not have the time to fully implement as requested. I initially thought I would have the time, but I quickly realized that it was more work than I could justify at the moment. I originally wrote the feature outside of the library, so I am using it in that manner for now.

If I have some time, I will fully implemented it as required. I think it is a useful feature that many people would use. I much rather declare my sitemap in code than xml.

[NightOwl888]

Agreed, it would be very useful for those who want a semantic markup alternative to XML and also will make database-driven ISiteMapNodeProvider implementations easier and more intuitive to implement.

Anyway, it is looking more like I have an immediate patch release which I will try to do in the next day or so followed by the minor version release. A few days for the minor version release might end up being a few weeks because there are at least half a dozen issues besides this one (only 1 of which I have completed) that are ripe to go into it.

This API isn't quite as pressing so if there is time I will attempt to integrate what you have and transform it into a more integrated solution, but being that it is a new API, I can see this could easily take a week or so to polish. Due to semantic versioning constraints, an interface can't be changed after it is released into the wild (but can be extended), so it would definitely need some extra time just to ensure it is intuitive to use and it fulfills all of the different ways that MvcSiteMapProvider could be configured, not to mention it will require some time put into documenting the members for intellisense.

I will leave the ball in your court for now and if I have the time to work on this I will follow up here to check whether you have found some time to work on it so we don't duplicate our efforts.

[NightOwl888]

FYI - I have started working with this and I think I have found a new direction. So far I don't have much yet, but I am starting to see a path that might work.

The direction you went was alright, but it would be better if the API conveyed some of the business rules about how MvcSiteMapProvider can be configured. For example, setting a title is required and only a route or a URL should be configured on a single node, not both. Parent key should only be valid on a base node, not any descendant nodes or it would be confusing. Single() should only be on the base node, but ToList() should be on both the base and the descendant nodes.

Also, certain properties have a specialized purpose, which would be best grouped on the same interface that is meant for a single purpose (display, routing, SEO, URL resolution, interop, etc.), and then shown as a group that can be set together.

Anyway, suffice to say I am working with this. Hopefully I will have something to show in a few days - but now I am focusing more on the language and structure of the API than the functionality of it.

[NightOwl888]

I now have non-functional demo of the fluent API that is intended to be used to get feedback. The source code is also online here (built on top of v4.5, so there are lots of untested changes). There is still a lot of refactoring to do, but it would be best to wait until after we have some feedback as to the direction to take the API.

The objective of the fluent API is to make it easy to both configure and to read the configuration, but these goals sometimes conflict so we need to find the best balance.

Try it! Make sure you actually create some nodes and node trees so you can see how the API flows.

Business Rules

Some of the business rules are baked into the API to make it easier to configure. Here is a list.

1. A node can have 4 types:
   a.) Matching Route
   b.) Matching Url
   c.) Dynamic Node Provider Definition
   d.) Grouping node
2. Route nodes must have a title and must have route values.
3. Url nodes must have a title and must have a url.
4. A dynamic node provider must provide the name of the provider and can have configured values that will be inherited by the dynamic nodes.
5. A grouping node must have a title.
6. A node can have a canonical key or a canonical URL, but not both.
7. A descendant node can inherit route values from the parent node.
8. A descendant node can inherit custom attributes from the parent node.
9. A descendant node cannot specify a parent key (because it is done semantically already).
10. From the base node (root of the current tree) you can specify to create an IEnumerable by specifying ToList().
11. From the base node you can specify to return just the base node by specifying Single().
12. Node trees can be chained together using ToList().Union().
13. ToList() and Single() do not appear until all of the required values have been entered.

Note that when using MatchingRoute(), the required controller and action values are not currently enforced. I am still contemplating how best to deal with node value inheritance (suggestions welcome).

Questions To Answer

1. Should we support dynamic nodes?

There are several reasons why we might not want to include this in the fluent API:

1. The "definition node" that is not added to the SiteMap is confusing.
2. The API is much more complex with dynamic node support.
3. It doesn't make much sense to use IDynamicNodeProvider in conjunction with ISiteMapNodeProvider because ISiteMapNodeProvider offers the same functionality and more. The only limitation is that ISiteMapNodeProvider currently only works with external DI.
4. If we support dynamic nodes within ISiteMapNodeProvider, the API will be more difficult to port to IDynamicNodeProvider because dynamic nodes cannot be defined on dynamic nodes.

The only argument I can think of to keep this support is:

1. It provides a semantic "placeholder" to remind you that there are children attached to the node from another source.

2. How do we deal with node value inheritance?

Some options:

1. Supply settings before creating the base node (in the RegisterNode() method) to explicitly define what values to inherit. Of course, many don't make much sense, such as title, description, key, etc, for which there would be no settings.
2. Implicitly inherit controller and action with the option to explicitly inherit other RouteValues, the same way we did in XML. This could be extended to include everything in the "WithInheritableRouteValues" subsection.
3. Inherit only values explicitly specified on the current node. This could make the inheritance logic more readable and understandable, but would also make the configuration more verbose.

What values should be inherited? What values should not be?

3. Is the API easily readable and easily writable?

These are important to get answered ASAP because once we release the API into the wild it will be difficult to change it without breaking people's configurations. I think we should eventually release the fluent API into the production version, but consider it BETA until we are relatively certain there will be no more major changes.

1. Should we use "With" prefixing the property setters (.WithTitle("My Title")), should we leave it off, or should we only put it on the "Starter" methods for each group and leave it off within the group (.WithDisplayValues(x => x.Title("My Title")))? The reason for using "With" is to make it flow into a sentence (.RegisterNode().MatchingRoute(x => x.WithController("Home"))).
2. Are there any improvements we can do to make the language used more understandable?
3. Should we use RegisterNode(), BuildNode(), or something else for the starter method? Unfortunately, CreateNode() is already being used by the existing API with a different return type, so we can't use that.

NOTE: We deliberately didn't follow the existing naming conventions if it didn't make sense. For example "PreservedRouteParameters" is now "AlwaysMatchingKey" to more accurately indicate what it is for.

NOTE: There is currently no intellisense documentation on the API, but when releasing to BETA there will be helpful pointers on what to put into the fields.

IMPORTANT: The API doesn't actually work yet - it is only an interface with no actual implementation. We need to hammer down the interface first before creating a working prototype.

[maartenba]

Trying to gather some feedback through Twitter...

[pauldotknopf]

Looks very good. Far better than what I proposed. Thanks!

[NightOwl888]

@theonlylawislove

What do you think of the idea of dropping support for dynamic node providers from this API? Personally, I think of IDynamicNodeProvider as the "old way" since ISiteMapNodeProvider was created, although there are no plans to drop support for it entirely from MvcSiteMapProvider. When using external DI, there is really no reason to use IDynamicNodeProvider because ISiteMapNodeProvider covers all of the same use cases. The only possible "advantage" is the fact that it is defined semantically so it reminds you that there are child nodes. But then, a code comment could potentially do that if a reminder were really needed. Thoughts?

[pauldotknopf]

I agree. It probably won't be used. It makes sense for XML only I think, since you can't dynamically create .xml files.