JsonDataListSource should allow relative URLs to be fetched via http, not local file system

[tompipe]

As far as I can tell, the JsonDataListSource currently assumes any urls prefixed with http are to be downloaded with the WebClient,

umbraco-contentment/src/Umbraco.Community.Contentment/DataEditors/DataList/DataSources/JsonDataListSource.cs

Line 192 in 7a104c1

if (url.StartsWith("http", StringComparison.InvariantCultureIgnoreCase) == true)

and all others are to be fetched from a file on the local file system.

umbraco-contentment/src/Umbraco.Community.Contentment/DataEditors/DataList/DataSources/JsonDataListSource.cs

Lines 206 to 209 in 7a104c1

	else
	{
	// assume local file
	var path = IOHelper.MapPath(url);

In the event the json is to be sourced from an api controller running on the same umbraco instance, then the url entered into the contentment editor needs to be an absolute url starting with http, which works perfectly of course.

However, in cases where the codebase is synced between different environments and the url changes, then this setting must be manually updated with the new environments url. Whilst not a showstopper of course, it's just something which is easily overlooked.

Perhaps a toggle option could be added, giving the choice of whether a relative path is to be fetched via http or from the file system?

Thought I'd log the suggestion and get your thoughts before I make a PR when I next have a free moment.

[leekelleher]

@tompipe, thanks for raising this.

Thinking through the various options, that immediately come to mind...

1. Why not make a custom data-source? (e.g. devil's advocate 👿) You know exactly what you need. It'll save on the overhead - saves making a HTTP request, saves parsing JSON. Could it be coded directly with a custom data-source? and if the code is only available in a controller, (for whatever reasons), it is possible to extract or refactor the logic?

2. Amend JSON data-source configuration. Which I think is what you're suggesting? Adding a field to explicitly state whether it's a HTTP request or local filesystem. Also I need to consider backwards-compatibility. I already feel there are too many configuration options for this data-source, (same with CSV and XML ones). I'm least keen on this idea.

3. Amend JSON data-source code/logic. I could make an assumption about intent of the URL input. e.g. if the URL starts with "http://" or "https://" then it's a HTTP request. If it starts with "~/" then it's local filesystem (mapped to a relative folder). If it starts with "/" (just a slash, without any prefix), then I could first check the local filesystem, otherwise fallback on a local HTTP request?

4. Placeholder tokens? Getting a bit more leftfield. I could check the URL string for tokens such as {HTTP_HOST} and {HTTP_PROTOCOL} (taking these names from Request.ServerVariables). e.g. so having... "{HTTP_PROTOCOL}://{HTTP_HOST}/umbraco/api/controller/action" would then be portable between environments. (I already do things like this for things like my own implementations of robots.txt/sitemap.xml logic).

Hmmm, ideas, ideas. 🤔

My preferred option is the first, as it reduced on the extra HTTP request, and that I don't need to make any code changes. 😆

Totally open to any other suggestions.

[hfloyd]

As per my own experience (detailed below), I think that I do agree with option 1.
I think it is the nature of developers to automatically go towards the things we know how to do already (and not read the docs unless we have a problem!)

My suggestion would be to surface that option a bit more in the dialog (with a link to the docs, perhaps?)

Keeping in mind that some devs will want to hit a local API for whatever reason, regardless of effort or httpRequests... I also agree with option 3. I think that strikes the right balance of assumptions. In general, when I see "~", it reminds me more of a physical file, whereas "/" indicates a root-relative url path.

[hfloyd]

I'll admit that, like @tompipe, my immediate thought was to create a local webapi to return the JSON... Primarily because I have done such a thing many times, and knew exactly how to get it up and running in minutes. Also, I was able to create a simple little model to return the right format of data, and figured it would be easy to create other JSON datasources using the same little model, and a similar api function format.

My code, in case you are curious:

    using System.Collections.Generic;
    using System.Linq;
    using System.Text;
    using System.Net.Http;
    using Newtonsoft.Json;
    using Umbraco.Web.WebApi;

    [IsBackOffice]
    public class DataListJsonApiController : UmbracoAuthorizedApiController
    {
        /// /Umbraco/backoffice/Api/DataListJsonApi/PageTypes
        [System.Web.Http.AcceptVerbs("GET")]
        public HttpResponseMessage PageTypes()
        {
            var allData = new List<DataListItem>();

            foreach (var ct in Services.ContentTypeService.GetAll().Where(n => !n.IsElement).OrderBy(n => n.Name))
            {
                if (ct.AllowedTemplates.Any())
                {
                    var item = new DataListItem();
                    item.Name = ct.Name;
                    item.Alias = ct.Alias;
                    item.Icon = ct.Icon;
                    item.Description = ct.Description;

                    allData.Add(item);
                }
            }

            string json = JsonConvert.SerializeObject(allData);

            return new HttpResponseMessage()
            {
                Content = new StringContent(
                    json,
                    Encoding.UTF8,
                    "application/json"
                )
            };
        }
    }

    public class DataListItem
    {
        [JsonProperty("name")]
        public string Name { get; set; }

        [JsonProperty("alias")]
        public string Alias { get; set; }

        [JsonProperty("icon")]
        public string Icon { get; set; }

        [JsonProperty("description")]
        public string Description { get; set; }
    }

However, considering @leekelleher 's very valid point about unnecessary http requests... I figured I'd try his suggestion of just making the custom DataSource directly.

Using the simple TimeZones example provided, I was able to quickly get it mocked up and the Contentment DataListItem model is basically the same as the one I had created for my webapi. The only issue I came across immediately is that UmbracoApiControllers have Services, UmbracoHelper, etc. available... whereas this does not:

I freely admit that I am still VERY fuzzy on the Dependency Injection pattern for Umbraco 8, but I wonder if there is any way that those assets could be "pre-injected" or whatever for the IDataListSource ?

I was able to fix it via the anti-pattern:

Honestly, I think this is the right way to go... Look at this sexiness:

And a preview too!

Here is my "converted" code:

    using System.Collections.Generic;
    using System.Linq;
    using Umbraco.Community.Contentment.DataEditors;
    using Umbraco.Core.PropertyEditors;
    using Umbraco.Web.Composing;

    public class PageTypesDataSource : IDataListSource
    {
        public string Name => "Page Types";

        public string Description => "All Content Types which are not Elements and have Templates";

        public string Icon => "icon-blueprint";

        public OverlaySize OverlaySize => OverlaySize.Small;

        public Dictionary<string, object> DefaultValues => default;

        public IEnumerable<ConfigurationField> Fields => default;

        public IEnumerable<DataListItem> GetItems(Dictionary<string, object> config)
        {
            var contentTypeService = Current.Services.ContentTypeService;
            var items = new List<DataListItem>();

            foreach (var ct in contentTypeService.GetAll().Where(n => !n.IsElement).OrderBy(n => n.Name))
            {
                if (ct.AllowedTemplates.Any())
                {
                    var item = new DataListItem();
                    item.Name = ct.Name;
                    item.Value = ct.Alias;
                    item.Icon = ct.Icon;
                    item.Description = ct.Description;

                    items.Add(item);
                }
            }

            return items;
        }
    }

[leekelleher]

@hfloyd Awesome, I'm happy to hear the custom data-source worked for you. 🙌

With the Current.Services.ContentTypeService line. That's where the Dependency Injection comes in. (I'm also quite fuzzy with how it all works too).

Here's an example of what I've done on the Umbraco Content data-source...

https://github.com/leekelleher/umbraco-contentment/blob/1.4.2/src/Umbraco.Community.Contentment/DataEditors/DataList/DataSources/UmbracoContentDataListSource.cs#L24

In the class's constructor, add a parameter IContentTypeService contentTypeService and assign it to a private readonly field. You can then use that service inside the GetItems() method. Umbraco (LightInject) does all the automagic wiring up.

private readonly IContentTypeService _contentTypeService;

public PageTypesDataSource(IContentTypeService contentTypeService)
{
    _contentTypeService = contentTypeService;
}

// then use `_contentTypeService.GetAll()` later on.

In terms of Umbraco v9 (.NET Core) you're going to see a LOT of Dependency Injection code, it's worth experimenting with it in v8.

[DanDiplo]

@leekelleher Any idea how you can inject your own non-Umbraco dependencies in here? I've tried injecting a dependency in the constructor but LightInject throws a wobbler and Umbraco can't boot. It's registered in a composer and the same service works fine when injected into Controllers in the site etc.

In my case I'm attempting to inject IJobDataService - a truncated version of how I'm doing it is shown below:

public class AtsTeamDataSource : IDataListSource
{
   private readonly IJobDataService dataService;

   public AtsTeamDataSource(IJobDataService dataService)
   {
         this.dataService = dataService;
   }
}

As I say, the service is registered in a IUserComposer in the same project, and it works elsewhere:

composition.Register<IJobDataService, JobDataService>(Lifetime.Scope);

The basic error is:

Boot failed: Umbraco cannot run. See Umbraco's log file for more details.

-> Umbraco.Core.Exceptions.BootFailedException: Boot failed.

-> System.NullReferenceException: Object reference not set to an instance of an object.
at Umbraco.Web.Runtime.WebInitialComposer.<>c.b__0_5(IFactory factory) in D:\a\1\s\src\Umbraco.Web\Runtime\WebInitialComposer.cs:line 100
at Umbraco.Core.Composing.LightInject.LightInjectContainer.<>c__DisplayClass20_0`1.b__0(IServiceFactory f) in D:\a\1\s\src\Umbraco.Core\Composing\LightInject\LightInjectContainer.cs:line 172...

NB. It does work if I use the "dreaded" service-locator pattern ie.

var dataService = Umbraco.Core.Composing.Current.Factory.GetInstance<IJobDataService>();

Sorry, this is off topic, but just wondered if you had any thoughts on constructor injection?

[leekelleher]

Hi @DanDiplo. I haven't tried registering/injecting my own services into a custom data-source. But @tompipe something similar on #83. I think the solution was to wrap the constructor's parameter in a Lazy<>, to delay the loading.

My knowledge of DI is limited, I end up hacking around until something works. 😬

[DanDiplo]

@leekelleher Thanks, that idea of wrapping in a Lazy<> works! Genius 👍 So, for the record and to help anyone else who stumbles on this, what I do in the constructor is this now:

private readonly Lazy<IJobDataService> dataService;

public AtsTeamDataSource(Lazy<IJobDataService> dataService)
{
    this.dataService = dataService;
}

public IEnumerable<DataListItem> GetItems(Dictionary<string, object> config)
{
    var items = this.dataService.Value.GetMyData().Select(x => new DataListItem()
    {
        Name = x.Name,
        Value = x.Value,
        Icon = "icon-server-alt"
    });

    return items;
}

I'm like you - I find DI to be a kind of dark magic that offers a lot of power, but also lets me shoot myself in the foot with a fully loaded automatic weapon because I don't understand how it all works. But I gather that this issue was basically being caused by the Contentment composer running before my User Composer, which makes sense. I'm just glad I didn't have to figure it all out :)

[hfloyd]

Thanks, @leekelleher!
🎇That Automagically worked. 🎆

Updated code here: https://gist.github.com/hfloyd/a6f3772420b7c1386175ae0797812300

[soreng]

@leekelleher I was thinking if the json-file should be pickable with the filesystem picker used in the blockeditor.
I know it's a bit counterintuitive to the discussion on supporting endpoints outside (but relative to) the app.