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

bookmarkFinder.FindBookmarksAsync<Activity>(correlationId) returns all of that type #1604

Closed
matt4446 opened this issue Oct 5, 2021 · 19 comments · Fixed by #1634
Closed
Labels
bug Something isn't working prio high Is on the roadmap for the near-future
Milestone

Comments

@matt4446
Copy link
Contributor

matt4446 commented Oct 5, 2021

Finding bookmarks by correlation id doesn't seem to filter by correlation id;
correlationId : Bookmarks-2021-10-05 15:48:34Z

var all = await bookmarkFinder.FindBookmarksAsync<Activities.RunCodeActivity>(correlationId: correlationId);

image

The two records above being two different correlations roughly ~2 mins from each other where I should only be expecting the latter.

Speaking of bookmarks...
They never seem to go into the yessql store. Is there an option to persist them that I've missed?
image

And they all disappear when the app restarts

@sfmskywalker sfmskywalker added the bug Something isn't working label Oct 6, 2021
@sfmskywalker sfmskywalker added this to the Elsa 2.4 milestone Oct 6, 2021
@sfmskywalker sfmskywalker added the prio high Is on the roadmap for the near-future label Oct 6, 2021
@sfmskywalker
Copy link
Member

Different but related issue. Before my fix, the in-memory provider was used for Bookmark storage but now the YesSQL one is used. Looks like you found a query that is not properly mapped. If you can show me the full stack trace I might see what code is setting up the OR specification.

@sfmskywalker sfmskywalker reopened this Oct 7, 2021
@matt4446
Copy link
Contributor Author

matt4446 commented Oct 7, 2021

beginning at:
image

10/07/2021 12:20:30 +01:00 Unhandled exception rendering component: "An expression of non-boolean type specified in a context where a condition is expected, near 'or'."
Microsoft.Data.SqlClient.SqlException (0x80131904): An expression of non-boolean type specified in a context where a condition is expected, near 'or'.
   at Microsoft.Data.SqlClient.SqlCommand.<>c.<ExecuteDbDataReaderAsync>b__203_0(Task`1 result)
   at System.Threading.Tasks.ContinuationResultTaskFromResultTask`2.InnerInvoke()
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot, Thread threadPoolThread)
--- End of stack trace from previous location ---
   at Dapper.SqlMapper.QueryAsync[T](IDbConnection cnn, Type effectiveType, CommandDefinition command) in /_/Dapper/SqlMapper.Async.cs:line 418
   at YesSql.Store.ProduceAsync[T,TState](WorkerQueryKey key, Func`2 work, TState state)
   at YesSql.Services.DefaultQuery.Query`1.ListImpl()
   at YesSql.Services.DefaultQuery.Query`1.ListImpl()
   at Elsa.Persistence.YesSql.Stores.YesSqlStore`2.FindManyAsync(ISpecification`1 specification, IOrderBy`1 orderBy, IPaging paging, CancellationToken cancellationToken)
   at Elsa.Persistence.YesSql.Stores.YesSqlStore`2.FindManyAsync(ISpecification`1 specification, IOrderBy`1 orderBy, IPaging paging, CancellationToken cancellationToken)
   at Elsa.Services.Bookmarks.BookmarkFinder.FindBookmarksAsync(String activityType, IEnumerable`1 bookmarks, String correlationId, String tenantId, CancellationToken cancellationToken)

@sfmskywalker
Copy link
Member

I can't seem to reproduce this issue from here. Can you send me a (stripped down) version of your project I can look at?

@matt4446
Copy link
Contributor Author

matt4446 commented Oct 7, 2021

Yep, creating one now while working on other bugs

@matt4446
Copy link
Contributor Author

matt4446 commented Oct 8, 2021

Version: 2.4.0-preview.302
https://github.com/matt4446/ElsaQuickstarts.Server.DashboardAndServer2

I didn't come across the above error ("An expression of non-boolean type specified in a context where a condition is expected, near 'or'.") on a new project with yessql+sqllite so will look at my more complicated version more closely. Finding bookmarks with both these worked:
var currentBookmarks = (await approveInputDispatcher.FindRequiredInputs()).ToList();
var currentWithCorrelationId = (await approveInputDispatcher.FindRequiredInputs(first.CorrelationId)).ToList();

However, this function isn't returning anything in the test project above.:

https://github.com/matt4446/ElsaQuickstarts.Server.DashboardAndServer2/blob/ef9cebf23d1b04e873e329572de04e02edc9fc2e/ElsaQuickstarts.Server.DashboardAndServer2/Services/ApproveInputDispatcher.cs#L39

It returns 0 (IEnumerable) records, if you can spot anything wrong with it? I tried a few combinations with no success

@matt4446
Copy link
Contributor Author

matt4446 commented Oct 8, 2021

The example above sets the workflow into action itself

Producing new work every 30 seconds
And finding that work every 10 seconds with the bookmarks to potentially approve them.

@matt4446
Copy link
Contributor Author

matt4446 commented Oct 8, 2021

"An expression of non-boolean type specified in a context where a condition is expected, near 'or'." still occurs in my main project. More digging to do (same 302 version)
:)

@matt4446
Copy link
Contributor Author

matt4446 commented Oct 8, 2021

Its failed sql

exec sp_executesql N'SELECT DISTINCT [Elsa2_Bookmarks_Document].* FROM [Elsa2_Bookmarks_Document] INNER JOIN [Elsa2_Bookmarks_BookmarkIndex] AS [BookmarkIndex_a1] ON [BookmarkIndex_a1].[DocumentId] = [Elsa2_Bookmarks_Document].[Id] WHERE ((([BookmarkIndex_a1].[TenantId] IS NULL) and ([BookmarkIndex_a1].[ActivityType] = @p0)) and (@p1 or ([BookmarkIndex_a1].[CorrelationId] = @p2)))',N'@p0 nvarchar(4000),@p1 bit,@p2 nvarchar(4000)',@p0=N'RunCodeActivity',@p1=0,@p2=N'Bookmarks-2021-10-06 08:15:35Z'

Messages:

Msg 4145, Level 15, State 1, Line 1
An expression of non-boolean type specified in a context where a condition is expected, near 'or'.

Completion time: 2021-10-08T16:50:38.1811600+01:00

@matt4446
Copy link
Contributor Author

matt4446 commented Oct 8, 2021

And im missing the CorrelationId field. Might be because this DB was created in the alpha days.
image

@p1 being the problem by the looks of it.
(@p1 or ([BookmarkIndex_a1].[CorrelationId] = @p2))

@sfmskywalker
Copy link
Member

It returns 0 (IEnumerable) records, if you can spot anything wrong with it? I tried a few combinations with no success

It seems to work correctly when I try:

image

And im missing the CorrelationId field. Might be because this DB was created in the alpha days.

Yes that would indeed be an issue. You should be able to add it manually.

@matt4446
Copy link
Contributor Author

matt4446 commented Oct 9, 2021

The later problem was with the collect and execute 😁.
image

The bookmarking part seems to be functioning ok in the demo project.

@matt4446
Copy link
Contributor Author

matt4446 commented Oct 11, 2021

It would appear the following could be causing the bad SQL from Elsa.Persistence.Specifications.Bookmarks.BookmarkSpecification

(Expression<Func<Bookmark, bool>>) 
(bookmark => bookmark.TenantId == this.TenantId 
	&& bookmark.ActivityType == this.ActivityType 
	&& (
		this.CorrelationId == default (string) || bookmark.CorrelationId == this.CorrelationId)
);

image

It would appear this part is evaluating
(this.CorrelationId == default (string) || bookmark.CorrelationId == this.CorrelationId)
to 0 OR CorrelaitionId = "create farm...."

With the exception:
10/07/2021 12:20:30 +01:00 Unhandled exception rendering component: "An expression of non-boolean type specified in a context where a condition is expected, near 'or'."

SELECT DISTINCT [Elsa2_Bookmarks_Document].* FROM [Elsa2_Bookmarks_Document] INNER JOIN [Elsa2_Bookmarks_BookmarkIndex] AS [BookmarkIndex_a1] ON [BookmarkIndex_a1].[DocumentId] = [Elsa2_Bookmarks_Document].[Id] WHERE ((([BookmarkIndex_a1].[TenantId] IS NULL) and ([BookmarkIndex_a1].[ActivityType] = @p0)) and (@p1 or ([BookmarkIndex_a1].[CorrelationId] IS NULL)))

Now to check the test project

@matt4446
Copy link
Contributor Author

Interesting. The test project gets to the same place with
image

(Expression<Func<Bookmark, bool>>) (bookmark => bookmark.TenantId == this.TenantId && bookmark.ActivityType == this.ActivityType && (this.CorrelationId == default (string) || bookmark.CorrelationId == this.CorrelationId));

Something YesSql is doing differently I wonder.

@matt4446
Copy link
Contributor Author

I've updated the git repository to reproduce the problem. It required SQL Server 😂

SQL and parameters were equal between both SqlLite and SqlServer on both my projects (test and master).
It fails with "An expression of non-boolean type specified in a context where a condition is expected, near 'or'." on SQL Server but the same is fine on SqlLite using the YesSql providers.

We could create a better Expression<> from

(Expression<Func<Bookmark, bool>>) (bookmark => bookmark.TenantId == this.TenantId && bookmark.ActivityType == this.ActivityType && ( this.CorrelationId == default (string) || bookmark.CorrelationId == this.CorrelationId) );

bookmark.CorrelationId == this.CorrelationId should only be present if this.CorrelationId is null or empty.

@sfmskywalker
Copy link
Member

Makes sense. Do you want to send a PR?

@matt4446
Copy link
Contributor Author

Possibly, which direction would you prefer? Its been a while since I've used linqkit

public override Expression<Func<Bookmark, bool>> ToExpression() =>
            CorrelationId == null
            ? bookmark => bookmark.TenantId == TenantId
                && bookmark.ActivityType == ActivityType
            : bookmark => bookmark.TenantId == TenantId
                && bookmark.ActivityType == ActivityType
                && bookmark.CorrelationId == CorrelationId;
public Expression<Func<Bookmark, bool>> ToExpression() {
            LinqKit.ExpressionStarter<Bookmark>? expressionBuilder = LinqKit.PredicateBuilder.New<Bookmark>();

            expressionBuilder = expressionBuilder.And(bookmark => bookmark.TenantId == TenantId);
            expressionBuilder = expressionBuilder.And(bookmark => bookmark.ActivityType == ActivityType);

            if (this.CorrelationId != null) 
            {
                expressionBuilder = expressionBuilder.And(bookmark => bookmark.CorrelationId == CorrelationId);
            }

            return expressionBuilder;
        }

@sfmskywalker
Copy link
Member

Let's stick with your first suggestion - seems simpler if you agree?

@matt4446
Copy link
Contributor Author

It was certainly the quickest to write but not as much fun as playing with linqkit

@sfmskywalker
Copy link
Member

Well I certainly wouldn’t want to bereft you of your fun :D

Feel free to use linqkit - it may prove useful in other areas as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working prio high Is on the roadmap for the near-future
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants