U4-6696 - UmbracoHelper.GetPreValueAsString fires exception with invalid integer

Created by Flavio Spezi 12 Jun 2015, 17:28:44 Updated by Shannon Deminick 26 Jun 2015, 08:08:07

When I call Umbraco.GetPreValueAsString() method with id = 1 argument the method fires an exception.

If id argument is a hight invalid value (like id = 999999) the method returns an empty string.

System.InvalidOperationException: Sequence does not contains elements.

System.Linq.Enumerable.Single(IEnumerable1 source, Func2 predicate) +621 Umbraco.Core.Persistence.Repositories.DataTypeDefinitionRepository.GetPreValueAsString(Int32 preValueId) +300 Umbraco.Core.Services.DataTypeService.GetPreValueAsString(Int32 id) +105 AppLib.Logic.Support.ModelHelper.TrueFalseNullable(IPublishedContent content, String propertyAlias, Boolean recursive) ... ....

Comments

Craig Noble 14 Jun 2015, 13:46:35

I dont mind taking a look at this bug. Can this be set to assigned?


Craig Noble 16 Jun 2015, 22:51:36

I cannot replicate this issue. I have an empty string returned every time.


Flavio Spezi 17 Jun 2015, 06:21:36

The issue fires when Cache is not well filled: the method that fires exception is in Umbraco.Core.Persistence.Repositories.DataTypeDefinitionRepository.GetPreValueAsString, the first Single method; then it is important that the cached object is not null.

I found a method to reproduce it, but theese steps does not fires issue any time.

  • Go to Umbraco Backoffice/Developer/DataTypes.
  • Add new prevalue item in a Radio Button List DataType; then Save. This is necessary to clear cache.
  • Test this in a Razor page:
		<div>
			fail 1: @Umbraco.GetPreValueAsString(1)
		</div>
		<div>
			fail 2: @Umbraco.GetPreValueAsString(2)
		</div>
		<div>
			valid value: @Umbraco.GetPreValueAsString(- a valid value -)
		</div>
		<div>
			fail 4: @Umbraco.GetPreValueAsString(4) // this linecode fires exception
		</div>

In my test, fail 4 line does not fires exception any time. Sometimes it is necessary to run the sequence "known value" / "unknown value" more times, with other new values. Then you can try this:


        @{
			var rnd = new System.Random();
			for (var i = 0; i < 1000; i++) {
				var id = rnd.Next(0, 1000);
				@Html.Raw("<br/><div class=\"fld\">")
				@Html.Raw(string.Format("<div class=\"lbl\">{0}:</div>", id))
				try {
					var strVal = Umbraco.GetPreValueAsString(id);
					@Html.Raw(string.Format("<div class=\"val\">{0}</div>", strVal.ToJS()))
				} catch (Exception ex) {
					@Html.Raw(string.Format("<div class=\"val\"><code>Error: <pre>{0}</pre></code></div>", ex.ToString()))
					@Html.Raw("</div>")
					break;
				}
				@Html.Raw("</div>")
			}
		}


Craig Noble 24 Jun 2015, 21:18:39

@fspezi great. I can now replicate this.


Craig Noble 24 Jun 2015, 22:19:04

Currently fixing this.

It is because the cache is being searched via regex, which isn't being accurate enough. For example, if you have cached prevalue 17, and you request 1, which isn't cached, it will try and still find 17 and didn't find 1. So single is breaking.

This regex is the culprit (7 is example prevalue id): UmbracoPreVal[\d]+-[,]7[,\d$]

Which would have 7, 71, 72, 73 etc. So this prevents any fetching of other prevalues if it is not in the cache yet.

I will be submitting a new regex for this.


Flavio Spezi 25 Jun 2015, 07:27:33

Wonderful! Thanks!


Craig Noble 25 Jun 2015, 22:05:31

Pull request submitted

Here is a description of the problem:

When a prevalue is requested, it would initially fetch it from the database and cache it in a cache item along with other associated prevalues for the datatype. The key for the item would be UmbracoPreVal-87-21,33,48, where -87 is the datatype and 21,33,48 is the prevalue id's in the cache.

Example: If I get a prevalue with an id of 33, it would cache it. If I then request a prevalue with id 3, it would match UmbracoPreVal-87-21,33,48 because the regex would match the 3 in 33. The code finds the cached item and calls .Single when the prevalue doesn't exist for 3, and will throw an exception.

To fix this, I refactored the regex to match the exact prevalue id. So now only the cache item containing the prevalue would be returned and if not, it will proceed to the database as normal.


Priority: Normal

Type: Bug

State: Fixed

Assignee:

Difficulty: Normal

Category: Architecture

Backwards Compatible: True

Fix Submitted:

Affected versions: 7.2.6

Due in version: 7.3.0

Sprint:

Story Points:

Cycle: