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

Create a Contentful module #24

Open
daveaglick opened this issue Jun 16, 2015 · 7 comments
Open

Create a Contentful module #24

daveaglick opened this issue Jun 16, 2015 · 7 comments

Comments

@daveaglick
Copy link
Member

To pull content from https://www.contentful.com

@JasonKoopmans
Copy link

I'm interested in this module. Do you have thoughts on the item? I was thinking of using this client to the Content Delivery API. I have some concerns that a module that uses this client would be challenged in how to take the strongly-typed style for filtering/ordering of the client and expose it in the Wyam config.

See SearchFilters

@daveaglick
Copy link
Member Author

Awesome! I think you will find that I have lots of thoughts on just about everything (I tend to be very verbose) :)

I wasn't even aware that there were any .NET Contentful wrapper libraries. I had assumed the REST API would need to be used so this library already gives us a great head start.

First thought is in regard to providing an authentication token. I've given a little thought to this kind of scenario (see #73), but nothing is implemented yet. It may not be that important in this case since the productionAccessToken the library needs opens up the read-only Content Delivery API and not the more security-sensitive Content Management API.

I would envision the Wyam module constructor looking very similar to the Contentful.NET ContentfulClient constructor:

Pipelines.Add("Contentful",
    Contentful("productionAccessToken", "spaceId")
    // ...
);

I think you've identified the main challenge: the Contentful API is rich enough (as is the Contentful.NET implementation of it) that easily representing it in the module may be difficult. That said, I think the module should be as complex as it needs to be to represent the full set of use cases. In this case, we should aim to eventually support anything the Contentful API supports (or at least that our wrapper library exposes).

The pattern I've taken with most of the other modules is to chain together a series of fluent methods.

First thoughts on what a module might look like (totally spit-balling here - I'd be more than happy to consider alternate implementations)

public class Contentful : IModule
{
    private readonly IContentfulClient _client;
    private readonly string _id;  // If this is set we want a single item, throw exceptions for any fluent methods
    private readonly Type _contentfulItemType;
    private readonly List<SearchFilter> _searchFilters = new List<SearchFilter>();

    public Contentful(string productionAccessToken, string spaceId)
    {
          _client = new ContentfulClient(productionAccessToken, spaceId);
    }

    // An additional constructor to fetch a single specific item
    public Contentful(string productionAccessToken, string spaceId, string /* or int? */ id)
        this(productionAccessToken, spaceId)
    {
        _id = id;
    }

    // Might want some additional overloads for the single item constructor
    // that take a ContextContent and/or DocumentConfig instead of just a 
    // hard-coded id, that way the input documents can be used to specify 
    // which item(s) to fetch

    public Contentful SearchSpaces()
    {
        // Throw if _contentfulItemType is already set (can only search one type of thing)
        _contentfulItemType = typeof(Space);
    }

    // etc.

    public TThis WithEqualityFilter(...)
    {
        // Throw if _contentfulItemType isn't set yet
        // ...
    }

    public TThis WithNumericFilter(...)
    {
        // ...
    }

    // Might want some additional overloads for each filter fluent method
    // that take a ContextContent and/or DocumentConfig instead of just 
    // hard-coded properties and conditions, that way
    // the input documents can be used to specify the filter(s)

    public IEnumerable<IDocument> Execute(
        IReadOnlyList<IDocument> inputs,
        IExecutionContext context)
    {
        // Throw if _contentfulItemType isn't set
       // Throw if we don't have at least one filter
    }
}

I think it would be okay to represent the Contentful.NET BuiltInProperties as strings. I'm generally opposed to magic strings, but since we use them pretty extensively for metadata keys anyway, I see this just being another similar type of thing.

To wrap it all up, we'd end up with a config file that looks something like:

Pipelines.Add("Contentful",
    Contentful("productionAccessToken", "spaceId")
        .SearchSpaces()
        .WithEqualityFilter("ContentType", "cat")
    WriteFiles()
);

Of course, this is just one approach. I can envision several other ways to accomplish this. For example, to address similar config complexity the ImageProcessor module by @dodyg uses classes to contain extra config information instead of a purely fluent interface. I'd be open to any approach.

If you decide to tackle it, good luck! I'm here for any help you need, and I'll be watching with great interest. I think CMS-as-a-service is the next logical step for static generators, and Contentful looks to be one of the leading services in this space. Getting support in Wyam would be awesome and something of a coup since not many other generators are going in this direction yet.

@JasonKoopmans
Copy link

I'm up for it. You've definitely given me some good ideas to get started with it.

@JasonKoopmans
Copy link

I'm off to a good start. I have run into a snag mocking the IContentfulClient interface that's a part of the Contentful.Net package using NSubstitute. IContentfulClient has dynamic objects in its model that I would like to mock, and it seems NSub seems to have difficulty accommodating in this specific case. I've included links for reference. Despite there being suggested workarounds using the Extension methods, I still get Runtime Binding Exceptions on the setup code for a non-virtual property on a class that returns a dynamic. Its this combination that seems to be the problem.

If:

  • Interface instead of a class >> it would work
  • property is marked as virtual in class >> it would work
  • not a dynamic >> It would work - If the Entry.Fields property didn't return a dynamic, I could just mock the methods GetAync() and SearchAsync() on IContentfulClient

I don't have a commit pushed yet to illustrate, but this test seems to capture the essence of my problem. Its an adapted test that appears in nsubstitute/NSubstitute#75

Suggestions? Try another Mock framework to get better support for this case? Something else?

public interface IInterface
        {
            dynamic ReturnsDynamic();
        }
        public class HasDynamicProperty : IInterface
        {
            public dynamic ReturnsDynamic()
            {
                dynamic d = new ExpandoObject();
                d.Text = "Yes";
                return d;
            }
            public dynamic MyProperty { get; set; }
        }

        public class HasDynamicPropertyWithVirtual : IInterface
        {
            public virtual dynamic ReturnsDynamic()
            {
                dynamic d = new ExpandoObject();
                d.Text = "Yes";
                return d;
            }
            public virtual dynamic MyProperty { get; set; }
        }

        [Test]
        public void MethodReturnsDynamic()
        {
            var sut = Substitute.For<IInterface>();
            var ut1 = Substitute.For<HasDynamicProperty>();
            var ut2 = Substitute.For<HasDynamicPropertyWithVirtual>();

            dynamic d = new ExpandoObject();
            d.Text = "Test";

            // This seems to setup fine -- uses a method
            sut.ReturnsDynamic().Returns(d);

            // Both of these methods throw binding execptions on setup
            // They aren't virtual -- this is probably the issue
            //ut1.MyProperty.Returns(d);
            //ut1.ReturnsDynamic().Returns(d);

            // Both of these ARE marked as virtual -- their setups work
            ut2.ReturnsDynamic().Returns(d);
            ut2.MyProperty.Returns(d);

            var result = sut.ReturnsDynamic();

            Assert.That(result.Text, Is.EqualTo("Test"));
            Assert.That(ut2.MyProperty.Text, Is.EqualTo("Test"));
        }

nsubstitute/NSubstitute#75
nsubstitute/NSubstitute#144
nsubstitute/NSubstitute#143

@daveaglick
Copy link
Member Author

This gets well beyond my NSubstitute knowledge :)

I've got no problem bringing in another mocking framework for this module's tests if it would help or if you have a preferred one that you know better. I like to keep the dependencies down in the released components, but feel free to go to town with the test libraries. The most important things with tests in my mind are that they test what needs to be tested and that they're not so onerous to write that you don't write the needed tests.

@daveaglick
Copy link
Member Author

That said, I would like to stick to NUnit for the actual testing framework so that test runners don't get confused - but other than that, have at.

@JasonKoopmans
Copy link

This gets well beyond my NSubstitute knowledge :)

Me too. I fought for a while. NSub is new to me. I like it the syntax, but this has been a rocky start.

I've got no problem bringing in another mocking framework for this module's tests if it would help or if you have a preferred one that you know better.

I have a couple to try. Probably start with Moq

I like to keep the dependencies down in the released components, but feel free to go to town with the test libraries.

I do too. Its why I asked first. Thanks.

That said, I would like to stick to NUnit for the actual testing framework so that test runners don't get confused - but other than that, have at.

No problem here so far.

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

No branches or pull requests

2 participants