Allow describing an index from a given `Type`. #525

hyzx86 · 2023-12-28T16:09:44Z

Here is an example

public class PropertyDynamicIndexProvider : IndexProvider<Property>
{
    public override void Describe(DescribeContext<Property> context)
    {
        context
            .For(typeof(PropertyIndex))  // in addition to the existing ".For<PropertyIndex>()"
            .Map(property => new PropertyIndex
            {
                Name = property.Name,
                ForRent = property.ForRent,
                IsOccupied = property.IsOccupied,
                Location = property.Location
            });
    }
}

Support dynamic Index type

README.md

src/YesSql.Abstractions/Indexes/DescribeContext.cs

src/YesSql.Abstractions/Indexes/DescribeFor.cs

src/YesSql.Core/Commands/IndexCommand.cs

test/YesSql.Tests/Indexes/PropertyIndex.cs

test/YesSql.Tests/SqliteTests.cs

…nto dynamicIndexType

hyzx86 · 2023-12-29T06:58:56Z

Hi @MikeAlhayek ， All problems resolved

MikeAlhayek

@hyzx86 I simplified things. I also removed the update you had to the readme file. That can be part of the Wiki once this PR is merged.

hyzx86 · 2023-12-30T04:48:55Z

@MikeAlhayek Thanks for your help .
I didn't notice that this test should be moved to CoreTests because I copied it from one of the Sqlite Index tests above

MikeAlhayek · 2023-12-30T04:55:00Z

I guess since it's just to ensure the index is created using type, we could also have left it in the SQLite only. It does not matter either way. It does not hurt running it on all databases either

hyzx86 · 2023-12-30T04:57:46Z

src/YesSql.Core/Commands/IndexCommand.cs

@@ -67,20 +68,13 @@ protected static void GetProperties(DbCommand command, object item, string suffi

        protected static PropertyInfo[] TypePropertiesCache(Type type)
        {
-            if (TypeProperties.TryGetValue(type, out var pis))
+            if (TypeProperties.TryGetValue(type.FullName, out var pis))


This change is mainly to avoid a scenario
Because dynamically typed indexes are allowed in the code, there is no guarantee that the type will not change during use,
For example, when I save the type, it has 5 fields, and then I add a new field, then I need to regenerate the type index.
Two types of inconsistent errors occur

2023-12-29 10:42:33.4084|SalesProtalDev|00-91aa5d5c1f96b614b2ebf3f2e6dfdbd5-5d2a9c17352de5d4-00||OrchardCore.ContentManagement.DefaultContentManager|ERROR|IContentHandler thrown from OrchardCore.Contents.AuditTrail.Handlers.AuditTrailContentHandler by InvalidCastException System.InvalidCastException: [A]EasyOC.DynamicIndex.AmisSchemaDIndex cannot be cast to [B]EasyOC.DynamicIndex.AmisSchemaDIndex. Type A originates from 'DynamicTypesAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' in the context 'Default' in a byte array. Type B originates from 'DynamicTypesAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' in the context 'Default' in a byte array. at YesSql.Serialization.PropertyInfoAccessor.Invoker`2.Invoke(Object target) at YesSql.Commands.IndexCommand.GetProperties(DbCommand command, Object item, String suffix, ISqlDialect dialect) at YesSql.Commands.CreateIndexCommand.AddToBatch(ISqlDialect dialect, List`1 queries, DbCommand batchCommand, List`1 actions, Int32 index) at YesSql.Session.BatchCommands() at YesSql.Session.FlushAsync() at YesSql.Session.FlushAsync() at YesSql.Services.DefaultQuery.CountAsync() at YesSql.Services.DefaultQuery.QueryIndex`1.YesSql.IQueryIndex<T>.CountAsync() at OrchardCore.Contents.AuditTrail.Handlers.AuditTrailContentHandler.RecordAuditTrailEventAsync(String name, IContent content) at OrchardCore.Modules.InvokeExtensions.InvokeAsync[TEvents,T1](IEnumerable`1 events, Func`3 dispatch, T1 arg1, ILogger logger) at YesSql.Serialization.PropertyInfoAccessor.Invoker`2.Invoke(Object target) at YesSql.Commands.IndexCommand.GetProperties(DbCommand command, Object item, String suffix, ISqlDialect dialect) at YesSql.Commands.CreateIndexCommand.AddToBatch(ISqlDialect dialect, List`1 queries, DbCommand batchCommand, List`1 actions, Int32 index) at YesSql.Session.BatchCommands()

Because the index type may change while the program is running.

Just like this:

thrown from OrchardCore.Contents.AuditTrail.Handlers.AuditTrailContentHandler by InvalidCastException System.InvalidCastException: [A]EasyOC.DynamicIndex.AmisSchemaDIndex cannot be cast to [B]EasyOC.DynamicIndex.AmisSchemaDIndex. Type A originates from 'DynamicTypesAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' in the context 'Default' in a byte array. Type B originates from 'DynamicTypesAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' in the context 'Default' in a byte array.

There seems to be no way to reproduce this scenario in a unit test, or to reproduce this scenario would require a complicated process

…e cache.

hyzx86 · 2023-12-30T05:29:37Z

@MikeAlhayek I reupdated the code

        protected static PropertyInfo[] TypePropertiesCache(Type type)
        {
            if (TypeProperties.TryGetValue(type, out var pis))
            {
                return pis;
            }

+            // If the same name type is already cached, it should be removed from the cache.
+            var existsingType = TypeProperties.Keys.FirstOrDefault(x => x.FullName == type.FullName);
+            if (existsingType != null)
+            {
+                var existsingProps = TypeProperties[existsingType];
+                foreach (var prop in existsingProps)
+                {
+                    PropertyAccessors.Remove(prop, out _);
+                }
+
+                TypeProperties.Remove(existsingType, out _);
+            }

            var properties = type.GetProperties().Where(IsWriteable).ToArray();
            TypeProperties[type] = properties;
            return properties;
        }

Co-authored-by: Hisham Bin Ateya <[email protected]>

…nto dynamicIndexType

MikeAlhayek · 2024-01-02T17:59:23Z

@hyzx86

OrchardCore Is a multi-tenant system, it will certainly initialize IStore many times, which is not the problem of OC.

OC registers a singleton IStore for each tenant. During registering the IStore we register all indexes as you can see here
https://github.com/OrchardCMS/OrchardCore/blob/ef1eaab35f571fe9500dfe9099a6154fef0a4c9e/src/OrchardCore/OrchardCore.Data.YesSql/OrchardCoreBuilderExtensions.cs#L108-L114

so it wouldn't be creating multiple IStore instances. Maybe the issue is in something else.

The code have changes since it was approved

hyzx86 · 2024-01-03T01:18:49Z

OC registers a singleton IStore for each tenant. During registering the IStore we register all indexes as you can see here https://github.com/OrchardCMS/OrchardCore/blob/ef1eaab35f571fe9500dfe9099a6154fef0a4c9e/src/OrchardCore/OrchardCore.Data.YesSql/OrchardCoreBuilderExtensions.cs#L108-L114

so it wouldn't be creating multiple IStore instances. Maybe the issue is in something else.

Not really, pls see session.RegisterIndexes

MikeAlhayek · 2024-01-03T02:05:31Z

@hyzx86 have you tried implementing IIndexProvider interface for indexes that use generic type instead of ‘IIndexProvider`?

hyzx86 · 2024-01-03T04:18:28Z

@hyzx86 have you tried implementing IIndexProvider interface for indexes that use generic type instead of ‘IIndexProvider`?

Yes, but eventually I gave up
The TypeProperties cache actually holds The result type of the map function

IndexDescriptor<T, TIndex, TKey>.IndexType

It's only used when you delete an index, because the map index returns null when you delete an index, so you can't get its type

in Store.cs

Clearly limit the use of DescribeContext<> Maybe we can use a factory method here and implement it in the interface

        private static Func<IDescriptor> MakeDescriptorActivator(Type type)
        {
            var contextType = typeof(DescribeContext<>).MakeGenericType(type);
            return Expression.Lambda<Func<IDescriptor>>(Expression.New(contextType)).Compile();
        }

MikeAlhayek · 2024-01-03T04:55:14Z

If you use IIndexProvider then you would provide the type of index using ForType() I never tried it. But, this should work. From there you can caching the dynamic type not the type provided in IIndexProvider<T>

MikeAlhayek · 2024-01-03T05:26:50Z

If you need to create IndexTypeCacheProvider, the default implementation should solve the problem you are facing. Meaning your comment have to be addressed by default otherwise you're code can't be accepted since you are adding API that allows generic,

This interface needs to be reimplemented when dynamic type and multi-tenant scenarios are used,

You may want to add interface to the abstraction project maybe like this

public interface IIndexTypeCacheProvider
{
    PropertyInfoAccessor GetPropertyInfoAccessor(PropertyInfo property);

    PropertyInfo[] GetProperties(Type type);

    void Update(Type type);
}

The IndexTypeCacheProvider can go into the Core project.

But, I am not sure that the complexity added by IndexTypeCacheProvider is needed. Maybe @sebastienros can provide more feedback on the approach you taken.

hyzx86 · 2024-01-03T07:08:11Z

@MikeAlhayek updated the default index cache key

Now , you can modify the test PopulateIndexUsingGenericType

Comment on the following line

            // update index type cache
            store1.Configuration.IndexTypeCacheProvider?.UpdateCachedType(changedType);

And run the test ，Errors will be reproduced

There is nothing wrong with using the full name of the type as the Key for the cache.

As I mentioned before, dynamically indexed types may change the list of fields during the program run, so each time an indexed field is modified, a new type is generated.
We can call IndexTypeCacheProvider. UpdateCachedType(Type) updates the cached type after regenerating the type

However, in an OC or other project , it may be better to replace it with an in-memory cache or a distributed cache, since both of these are tenant isolated in an OC.

And we can define the full name of the class to start with the tenant name when using dynamic type generation

# Conflicts: # src/Directory.Packages.props

…nto dynamicIndexType

hyzx86 · 2023-12-30T05:02:37Z

src/YesSql.Core/Commands/IndexCommand.cs

            {
                return pis;
            }

-            // Clean up cache entries with the same name and different type
-            var exists = TypeProperties.Keys.FirstOrDefault(x => x.FullName == type.FullName);


To ensure that only meaningful types are cached, cached type with the same name will cleared

hyzx86 · 2023-12-30T05:05:30Z

src/YesSql.Core/Commands/IndexCommand.cs

@@ -67,20 +68,13 @@ protected static void GetProperties(DbCommand command, object item, string suffi

        protected static PropertyInfo[] TypePropertiesCache(Type type)
        {
-            if (TypeProperties.TryGetValue(type, out var pis))
+            if (TypeProperties.TryGetValue(type.FullName, out var pis))


There seems to be no way to reproduce this scenario in a unit test, or to reproduce this scenario would require a complicated process

hyzx86 · 2024-01-02T15:09:08Z

test/YesSql.Tests/Indexes/DynamicTypeGeneratorSample.cs

+            getterIL.Emit(OpCodes.Ldfld, fieldBuilder);
+            getterIL.Emit(OpCodes.Ret);
+
+


hyzx86 · 2024-01-02T15:09:15Z

test/YesSql.Tests/Indexes/DynamicTypeGeneratorSample.cs

+            PropertyBuilder propertyBuilder = typeBuilder.DefineProperty(propertyName, PropertyAttributes.None, propType, null);
+            FieldBuilder fieldBuilder = typeBuilder.DefineField("_" + propertyName, propType, FieldAttributes.Private);
+
+            // 使用 IL API 生成类型属性访问器


hyzx86 · 2024-01-02T15:09:19Z

test/YesSql.Tests/Indexes/DynamicTypeGeneratorSample.cs

+            // Return the dynamic type
+            return dynamicType;
+        }
+


hyzx86 · 2024-01-02T15:09:43Z

test/YesSql.Tests/Indexes/DynamicTypeGeneratorSample.cs

+using System.Reflection.Emit;
+using YesSql.Indexes;
+
+namespace YesSql.Tests.Indexes


hyzx86 · 2024-01-02T15:12:50Z

test/YesSql.Tests/CoreTests.cs

+            Assert.NotEmpty(testProperties2);
+
+        }
+


hyzx86 · 2024-01-03T06:50:09Z

src/YesSql.Abstractions/Indexes/IndexTypeCacheProvider.cs

+        }
+
+        protected bool IsWriteable(PropertyInfo pi)
+        {


@MikeAlhayek Because this method is needed for caching in Yessql, I changed the original IIndexTypeCacheProvider to IndexTypeCacheProvider and included some virtual methods

hyzx86 · 2024-01-03T06:52:11Z

src/YesSql.Abstractions/IConfiguration.cs

+        /// This interface needs to be reimplemented when dynamic type and multi-tenant scenarios are used,
+        /// using tenant separated cache media insteadexTypeCacheProvider 
+        /// </summary>
+        IndexTypeCacheProvider IndexTypeCacheProvider { get; set; }


@MikeAlhayek We need to add it to the IConfiguration so that it can be overridden by other implementations
So I extracted IndexTypeCacheProvider into the Abstraction project

Sorry for taking so long to reply. Because I didn't submit my comments 😢
I always wondered why there was a Pendding mark on my reply. I just figured it out.

# Conflicts: # src/Directory.Packages.props

hyzx86 · 2024-05-04T14:59:43Z

After a period of test run, it is found that this method is not reliable

Next I'm going to try to implement IIndexCommand through external code

sebastienros and others added 2 commits December 22, 2023 09:29

Use System.Text.Json as the default serializer (sebastienros#504)

9cbde32

remake DescribeContext<>.For

263f1d4

Support dynamic Index type

hyzx86 changed the title ~~remake DescribeContext<>.For Support dynamic Index type~~ Remake DescribeContext<>.For Support dynamic Index type Dec 28, 2023

This was referenced Dec 28, 2023

Implement document change tracking #521

Closed

Inject the IStore.IConfiguration into the DI to make it modifiable OrchardCMS/OrchardCore#14959

Closed

hyzx86 and others added 4 commits December 29, 2023 00:47

fix UnitTest

8fa1f8e

Update README.md

cc9cc7e

Update README.md

c6c65cd

Avoid InvalidCastException

d4645fa

hyzx86 changed the title ~~Remake DescribeContext<>.For Support dynamic Index type~~ Remake DescribeContext Support dynamic Index type Dec 29, 2023

fix type cache clear

2fd4d5b

MikeAlhayek requested changes Dec 29, 2023

View reviewed changes

hyzx86 and others added 4 commits December 29, 2023 14:15

Update README.md

23f9c6b

clear code style

25f4198

Merge branch 'dynamicIndexType' of https://github.com/hyzx86/yessql i…

e373ca0

…nto dynamicIndexType

Update PropertyIndex.cs

482ed90

hyzx86 requested a review from MikeAlhayek December 29, 2023 06:59

MikeAlhayek added 4 commits December 29, 2023 09:22

Simplify and Cleanup

f850380

Moving the test. Test should fail MySQL for verification.

86d4302

Fix MySQL error

74936c1

Last tewaks

2c69da9

MikeAlhayek previously approved these changes Dec 29, 2023

View reviewed changes

MikeAlhayek requested a review from sebastienros December 29, 2023 17:56

MikeAlhayek changed the title ~~Remake DescribeContext Support dynamic Index type~~ Allow describing an index from a given Type. Dec 29, 2023

hyzx86 commented Dec 30, 2023

View reviewed changes

If the same name type is already cached, it should be removed from th…

144d363

…e cache.

hyzx86 and others added 5 commits January 2, 2024 23:06

Update src/YesSql.Core/Commands/IndexCommand.cs

be8d8f3

Co-authored-by: Hisham Bin Ateya <[email protected]>

Update src/YesSql.Abstractions/Indexes/IndexTypeCacheProvider.cs

ebc0b32

Co-authored-by: Hisham Bin Ateya <[email protected]>

Update src/YesSql.Abstractions/Indexes/IndexTypeCacheProvider.cs

bc1d2e7

Co-authored-by: Hisham Bin Ateya <[email protected]>

Update src/YesSql.Abstractions/Indexes/DescribeContext.cs

72002c8

Co-authored-by: Hisham Bin Ateya <[email protected]>

Update src/YesSql.Abstractions/IStore.cs

16acf5e

Co-authored-by: Hisham Bin Ateya <[email protected]>

hyzx86 marked this pull request as ready for review January 2, 2024 15:15

hyzx86 requested review from MikeAlhayek and hishamco January 2, 2024 15:49

hyzx86 added 2 commits January 3, 2024 01:23

add UpdateTypeCache Sample

212a333

Merge branch 'dynamicIndexType' of https://github.com/hyzx86/yessql i…

c40413d

…nto dynamicIndexType

update default index cache

b0bf2ef

hyzx86 and others added 6 commits January 3, 2024 15:18

Restoring the test code

d5429ea

rename

cd0928a

update unit test

784b81c

Merge branch 'main' into release/5.0

88d1a56

# Conflicts: # src/Directory.Packages.props

merg from 5.x

ac2d719

Merge branch 'dynamicIndexType' of https://github.com/hyzx86/yessql i…

8e31c26

…nto dynamicIndexType

hyzx86 mentioned this pull request Mar 10, 2024

Request to support dynamic index type #505

Open

hyzx86 commented Apr 2, 2024

View reviewed changes

Merge remote-tracking branch 'origin/main' into dynamicIndexType

2b836e8

# Conflicts: # src/Directory.Packages.props

hyzx86 closed this May 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow describing an index from a given `Type`. #525

Allow describing an index from a given `Type`. #525

hyzx86 commented Dec 28, 2023 •

edited by MikeAlhayek

Loading

hyzx86 commented Dec 29, 2023

MikeAlhayek left a comment

hyzx86 commented Dec 30, 2023

MikeAlhayek commented Dec 30, 2023

hyzx86 Dec 30, 2023

hyzx86 Dec 30, 2023

hyzx86 Dec 30, 2023

hyzx86 commented Dec 30, 2023

MikeAlhayek commented Jan 2, 2024

hyzx86 commented Jan 3, 2024 •

edited

Loading

MikeAlhayek commented Jan 3, 2024

hyzx86 commented Jan 3, 2024 •

edited

Loading

MikeAlhayek commented Jan 3, 2024

MikeAlhayek commented Jan 3, 2024

hyzx86 commented Jan 3, 2024 •

edited

Loading

hyzx86 Dec 30, 2023

hyzx86 Dec 30, 2023

hyzx86 Jan 2, 2024

hyzx86 Jan 2, 2024

hyzx86 Jan 2, 2024

hyzx86 Jan 2, 2024

hyzx86 Jan 2, 2024

hyzx86 Jan 3, 2024

hyzx86 Jan 3, 2024

hyzx86 Apr 2, 2024 •

edited

Loading

hyzx86 commented May 4, 2024

		getterIL.Emit(OpCodes.Ldfld, fieldBuilder);
		getterIL.Emit(OpCodes.Ret);

Allow describing an index from a given Type. #525

Allow describing an index from a given Type. #525

Conversation

hyzx86 commented Dec 28, 2023 • edited by MikeAlhayek Loading

hyzx86 commented Dec 29, 2023

MikeAlhayek left a comment

Choose a reason for hiding this comment

hyzx86 commented Dec 30, 2023

MikeAlhayek commented Dec 30, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hyzx86 commented Dec 30, 2023

MikeAlhayek commented Jan 2, 2024

hyzx86 commented Jan 3, 2024 • edited Loading

MikeAlhayek commented Jan 3, 2024

hyzx86 commented Jan 3, 2024 • edited Loading

MikeAlhayek commented Jan 3, 2024

MikeAlhayek commented Jan 3, 2024

hyzx86 commented Jan 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hyzx86 Apr 2, 2024 • edited Loading

Choose a reason for hiding this comment

hyzx86 commented May 4, 2024

Allow describing an index from a given `Type`. #525

Allow describing an index from a given `Type`. #525

hyzx86 commented Dec 28, 2023 •

edited by MikeAlhayek

Loading

hyzx86 commented Jan 3, 2024 •

edited

Loading

hyzx86 commented Jan 3, 2024 •

edited

Loading

hyzx86 commented Jan 3, 2024 •

edited

Loading

hyzx86 Apr 2, 2024 •

edited

Loading