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

Provide a reference to FilterExpressionCompiler in the new CustomFilterCompiler parameter of type BinaryExpressionInfo #360

Closed
statler opened this issue Jul 8, 2019 · 9 comments

Comments

@statler
Copy link

statler commented Jul 8, 2019

In the recent update to 2.3, querying nested collections was supported using CustomFilterCompilers. It is my understanding that this requires explicit implementations for every different collection. This unfortunately does not deal with my requirement for a general collection/property query, but I think it will be really useful to improve the (solution I have implemented for my own purposes) - refer to my off topic comments here

The problem I have at the moment is one of maintainability as I heavily modified the FilterExpressionCompiler so I am running my own version of the asp.net data which is a pain. I think I can move all of my code out of there now into a CustomFilterCompiler and use the base repo, but the problem is my code relies on the other methods and properties of the FilterExpressionCompiler. Would it be possible to add a reference to the FilterExpressionCompiler that provides the BinaryExpressionInfo reference? For example

public interface IBinaryExpressionInfo {
    Expression DataItemExpression { get; }
    string AccessorText { get; }
    string Operation { get; }
    object Value { get; }
    FilterExpressionCompiler<T> BaseFilterExpressionCompiler 
}

Happy to hear any other suggested alternatives - but this sounds like the best way to compartmentalize my code out of your library...

@AlekseyMartynov
Copy link
Contributor

Even if we publish the FilterExpressionCompiler<T> class, your code won't see all its members. The only public method is LambdaExpression Compile(IList criteriaJson).

@statler
Copy link
Author

statler commented Jul 8, 2019

True, but is that immutable? If you could open up CompileCore that means we basically have access to everything in there...

That would make this REALLY powerful because we can then use it recursively (I think)

@AlekseyMartynov
Copy link
Contributor

Try call via Reflection. For example:

var filterCompilerType = Type
    .GetType("DevExtreme.AspNet.Data.FilterExpressionCompiler`1, DevExtreme.AspNet.Data")
    .MakeGenericType(typeof(MyClass1));

var filterCompilerInstance = Activator.CreateInstance(filterCompilerType, new object[] {
    false, // guardNulls
    false  // stringToLower
});

var compileCoreMethod = filterCompilerType.GetMethod("CompileCore", BindingFlags.NonPublic | BindingFlags.Instance);
var param = Expression.Parameter(typeof(MyClass1), "obj");
var filter = new object[] { "ID", 1 };

var expr = compileCoreMethod.Invoke(filterCompilerInstance, new object[] { param, filter });

If in the end you outline a required API surface, we'll consider to publish it via interfaces.

@statler
Copy link
Author

statler commented Jul 9, 2019

Yes - this would [mostly] work, but it requires me to have a type for MyClass1. My implementation that I use in that repo is completely generic based on the path to the property from the of FilterExpressionCompiler. As I understand it, using the above I would need to implement the CustomFilterCompiler for each different class's controller, whereas my filter works with your string based query language (slightly modified with the [] syntax).

I am not sure how my implementation will translate across into this CustomFilterCompiler at the moment, as I intercept the filter in CompileGroup and pull out any of my group filters and process them individually, before feeding them back into compilebinary - this is the only real change to your standard behaviour in FilterExpressionCompiler e.g. (new code is bold)

foreach(var item in criteriaJson) {
	var operandJson = item as IList;

	if(IsCriteria(operandJson)) {
		if(operands.Count > 1 && isAnd != nextIsAnd)
			throw new ArgumentException("Mixing of and/or is not allowed inside a single group");

		isAnd = nextIsAnd;
		if(operandJson.Count > 0) {
			bool isCollection = !(operandJson[0] is IList) && operandJson[0].ToString().Contains("[]");
			if(isCollection) CollectionCriteria.Add(item);
			else operands.Add(CompileCore(dataItemExpr, operandJson));
		} else CollectionCriteria.Add(Expression.MakeBinary(ExpressionType.Equal, Expression.Constant(true, typeof(bool)), Expression.Constant(true, typeof(bool))));
		nextIsAnd = true;
	} else {
		nextIsAnd = Regex.IsMatch(Convert.ToString(item), "and|&", RegexOptions.IgnoreCase);
	}
}

if(CollectionCriteria.Count > 0) {
	List CollectionOperands = GetCollectionOperands(dataItemExpr, CollectionCriteria, isAnd);
	operands.AddRange(CollectionOperands);
}

What would be awesome is if we could actually get a version of my filters into the asp.net data source. I know you don't like the "Products[].ProductName" syntax, but I am not wedded to it. You could just as easily test whether "Products" is a collection and just use "Products.ProductName" (I think), but I am not actually sure how best to do this.

Essentially, my code allows users to send ["Products[].ProductName","=","Awesome Computer"] or ["Products[].ProductName","contains","Computer"] (and other operators) without any custom code at all and it Just Works on every DataSourceLoader without any additional implementation. The only real problem with it for me is maintainability.

Would you consider integrating a solution like this into the code base? Then I don't need an API

If no, then I probably need a way to get the Type of the generic FilterExpressionCompiler.

Thanks :)

@AlekseyMartynov
Copy link
Contributor

My implementation that I use in that repo is completely generic based on the path to the property from the of FilterExpressionCompiler.

I used typeof(MyClass1) as an example. In your code, use typeof(T) or info.DataItemExpression.Type instead of a specific type.

I know you don't like the "Products[].ProductName" syntax, but I am not wedded to it.

It's not that we don't like the syntax or the feature. The ticket #277 is active. We are considering it as an idea for future enhancements, however it's not a priority.

Do CustomFilterCompilers and the reflection-based access to internal classes provide sufficient API to fulfill your app requirements?

@statler
Copy link
Author

statler commented Jul 15, 2019

Thanks for the reply Aleksey

The CustomFilterCompilers would provide the access I need except for the preprocessing I need to do to parse out my [] notation which happens in the groups processing. I would have to play around a bit to see if I can do it another way - its been a while since I have had my head in this code, and it will take a while to familiarise

Lets say I have enough to keep me busy here until I can get you an answer

@statler
Copy link
Author

statler commented Jul 15, 2019

BTW - my comments were marked off issue in that #277 thread, so is there somewhere else I need to flag this? I think my points from that thread are still relevant :)

@AlekseyMartynov
Copy link
Contributor

@statler I cleared the 'off-topic' flag from those comments. With the introduction of CustomFilterCompilers it became unfair. Thanks for your input.

@statler
Copy link
Author

statler commented Jul 16, 2019

Thanks Aleksey. Lets close this and leave the conversation on that ticket then.

@statler statler closed this as completed Jul 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants