Fix the QueriesDocument and the Queries UI

[MikeAlhayek]

Fix #16372

/cc @sebastienros @gvkries

[Piedone]

Why is splitting Queries into multiple documents necessary for the fix?

Also, please fix the build.

[MikeAlhayek]

@Piedone The problem with today's approach is that it stores all different query types in the same document. So when we add, update or remove a query, we serialize/deserialize the entire document. For this to work with STJ, we have to register each query type with the base type. When the feature is disabled after a query is added to the document, we get an exception because the derived type is no longer registered with the base type.

For example, if you enable the Sql feature, then add a SqlQuery using the UI. Now enable Lucene feature and disable the Sql feature. Visiting the queries UI will throw exception because the document has a record serialized to SqlQuery but there is no map between SqlQuery to Query.

To solve this problem, we agreed to remove the document manager during last meeting and instead to store each query as a document. This way we fix the issue and also avoid using the document manager or worry about registering additional types.

[gvkries]

Wow, what an effort. I knew why I left this to you 🤗

[Piedone]

@Piedone The problem with today's approach is that it stores all different query types in the same document. So when we add, update or remove a query, we serialize/deserialize the entire document. For this to work with STJ, we have to register each query type with the base type. When the feature is disabled after a query is added to the document, we get an exception because the derived type is no longer registered with the base type.

For example, if you enable the Sql feature, then add a SqlQuery using the UI. Now enable Lucene feature and disable the Sql feature. Visiting the queries UI will throw exception because the document has a record serialized to SqlQuery but there is no map between SqlQuery to Query.

To solve this problem, we agreed to remove the document manager during last meeting and instead to store each query as a document. This way we fix the issue and also avoid using the document manager or worry about registering additional types.

I see, thanks for explaining!

[Piedone]

I'll resume reviewing this in-depth once the exception is fixed. Also, please fix the tests.

[MikeAlhayek]

@Piedone the test should now pass. Feel free to check out the code changes.

[Piedone]

Would you like to check the code again, @gvkries?

[MikeAlhayek]

@gvkries this is the last PR that is blocking the release of 2. Do you have any additional feedback on this PR? Or, should we merge it?

Thank you

[gvkries]

@gvkries this is the last PR that is blocking the release of 2. Do you have any additional feedback on this PR? Or, should we merge it?

Thank you

Sorry, didn't had time the last couple of days. I will check ASAP.

[gvkries]

The design looks good to me so far, but when I save a SQL query, I loose it. There is a bug somewhere, I did not check in detail.

[gvkries]

Maybe we should add an extension method for registering an IQuerySource, it's already repeated several times here.

[MikeAlhayek]

done

[MikeAlhayek]

@gvkries I can’t seems to be able to reproduce that query not saving issue. Are you sure you are using the latest code?

[gvkries]

@gvkries I can’t seems to be able to reproduce that query not saving issue. Are you sure you are using the latest code?

I think so. Just create an new site with the blog recipe, go to Queries -> Edit and the SQL query gets deleted instead of modified.

[MikeAlhayek]

@gvkries I was finally able to reproduce the issue and fix it. Can you please confirm?

[gvkries]

LGTM, thanks @MikeAlhayek