remove FluentValidationExtensions.ValidateInstance

[benmccallum]

Looks like we had the same need haha. Builds off yours, includes tests.

[benmccallum]

Out of interest, how are you injecting a service into a validator? Constructor injection seems to be failing when AddValidatorsFromAssembly is called.

We're not covering that in test cases, but I might add one so we can.

[benmccallum]

I guess here needs to be updated to pull from the IoC container... , but then it'd have to build it and hope that it's got everything in it by that point.

GraphQL.Validation/src/GraphQL.FluentValidation/ValidatorTypeCache.cs

Lines 62 to 95 in 0836613

	/// Add all <see cref="IValidator"/>s in <paramref name="assembly"/>.
	/// </summary>
	public void AddValidatorsFromAssembly(Assembly assembly, bool throwIfNoneFound = true)
	{
	Guard.AgainstNull(assembly, nameof(assembly));
	ThrowIfFrozen();
	var assemblyName = assembly.GetName().Name;
	var results = AssemblyScanner.FindValidatorsInAssembly(assembly).ToList();
	if (!results.Any())
	{
	if (throwIfNoneFound)
	{
	throw new Exception($"No validators were found in {assemblyName}.");
	}
	return;
	}
	foreach (var result in results)
	{
	var validatorType = result.ValidatorType;
	if (validatorType.GetConstructor(new Type[]{}) == null)
	{
	Trace.WriteLine($"Ignoring ''{validatorType.FullName}'' since it does not have a public parameterless constructor.");
	continue;
	}
	var single = result.InterfaceType.GenericTypeArguments.Single();
	if (!typeCache.TryGetValue(single, out var list))
	{
	typeCache[single] = list = new List<IValidator>();
	}
	list.Add((IValidator) Activator.CreateInstance(validatorType, true));
	}

Else, I guess the ValidatorTypeCache would need to be re-architected to use the DI container right? Or more precisely the configured resolver for GraphQL I guess? Hmm...

[SimonCropp]

currently none of our validators use DI. so havnt had to think about it yet

[SimonCropp]

isnt this feature already covered by https://github.com/SimonCropp/GraphQL.Validation/blob/master/src/GraphQL.FluentValidation/FluentValidationExtensions_GetArgumentAsync.cs ?

[benmccallum]

I can work around in the immediate future, as I just need to pull a value to use in the validation, but would be good to hear your thoughts on what a potential strategy might be, and I can have a crack at it. Unless you feel inspired yourself.

In the meantime, if you're happy with this, I'd love if you could get it merged and released so I can turn it on tomorrow.

Cheers mate.

[benmccallum]

I need this extension method as I call it to do a validation on a complex object, with complex properties too, as per the unit test. Calling your extension methods there would be ideal, but you're just assuming a flat object whereas mine aren't. I use JsonConvert to read it out into an object then fire the validate myself on it.

I thought about adding to your extension methods what I'm doing, but it's probably an advanced scenario at then adds a dep on a json deserializer. Hence this PR to just expose an async version of what I'm currently using.

[SimonCropp]

fair enough. can u add some doco here https://github.com/SimonCropp/GraphQL.Validation/blob/master/readme.source.md then i will merge and release

[SimonCropp]

i just checked. and it seems complex types should be usable with GetValidatedArgumentAsync 5b9b4a4

[SimonCropp]

just to be clear. i dont object to the PR. but i want to better understand the use case, and possible add that as doco and a test scenario

[SimonCropp]

bump

[benmccallum]

Sorry @SimonCropp, I haven't forgot about this, just been waylaid with other stuff. Hopefully I can look this afternoon, else it'll be tomorrow (France time).

[benmccallum]

Seems like it is supported... weird. Looks like in master this commit may have made some improvements, but it's not clear to me when that was released. Could be a 3.x only but was backported to 2.x... who knows.

Will pull out those extension methods entirely then I guess, but leave the tests.

[benmccallum]

Alright, have removed the ValidateInstanceAsync I'd added and the ValidateInstance that I'd requested be added previously as at the time it definitely seemed to be needed for deserialising with nested objects in the Input types.

Have left and updated the tests to avoid any regressions.

[benmccallum]

I guess this is going to be a major version bump as it's technically breaking for anyone who was using the method we'd previously exposed. I'll leave that for you to bump while merging.

[benmccallum]

One side-effect of these going now is that I lose the flexibility to workaround the lack of DI-support in validators that I mentioned earlier. But that's another issue to solve and I'll just do some dodgy inline validation in the resolver for now to get around that. I'll raise an issue for us to discuss it separately.

[SimonCropp]

@benmccallum thanks for all the effort on this. and sorry about the delayed interactions