SchemaHelper.GetSchemaName won't work for a customized dbstore

[yueyinqiu]

And it's used widely. For example in AddAsync we can't indicate another schema to use at all.

[magiccodingman]

Mind explaining a bit more? Because right now you can do this:

[MagicTable("Person", DbNames.Client)]
public class Person

Which uses, "public MagicTableAttribute(string schemaName, string databaseName = null)"

So that attribute on the class you can alter the name and database name.

Or you saying that you want to work with a schema that you specifically don't have a C# model for so there's no attribute on a class that can let you indicate what to add it too?

[yueyinqiu]

Yes, I mean sometimes we don't want a C# model, but just a normal class. And perhaps some may like to share a same C# model for two or more databases/stores.

[magiccodingman]

This is really interesting because I've never had that use case come up before! I'd love to understand more about how you're approaching this. Right now, the system is built around hard-typed models, which is standard for both code-first and database-first LINQ to SQL approaches. That’s why repository patterns exist in C#, and this system follows a similar philosophy.

The AddAsync part itself isn't too difficult to modify, but the real challenge I see is in Where statements. Since LINQ relies on knowing the model type, how would dynamic filtering work in your case? Are you generating dynamic expressions at runtime for queries?

Would it be acceptable if AddAsync, GetById, and similar operations worked without a hard-typed model, but Where still required a defined type?

[yueyinqiu]

Oh, I feel a little hesitant now... Actually I created this issue when setting up unit tests.

We don't have any restrictions when creating the database. For example we could do:

        _ = await magic.OpenAsync(new DbStore()
        {
            Name = "OpenTest.StoreSchema",
            Version = 1,
            StoreSchemas =
            [
                new StoreSchema()
                {
                    Name = "S1",
                    Indexes = ["I11", "I12"],
                    PrimaryKey = "P11",
                    PrimaryKeyAuto = true,
                    UniqueIndexes = ["U11", "U12"]
                },
                new StoreSchema()
                {
                    Name = "S2",
                    Indexes = [],
                    PrimaryKey = "P21",
                    PrimaryKeyAuto = false,
                    UniqueIndexes = ["U21"]
                }
            ]
        });

However we cannot access those stores later if there are no such class linked with the StoreSchema.

I think whether we support it or not, at least we should be consistent in creating and accessing it. Since where can't be used without such a class, for me, perhaps the ideal solution is to make the constructor of StoreSchema private/internal.

[yueyinqiu]

By the way, when we open a database like this, the given database name in MagicTable is not respected:

[MagicTable("Records", "DATABASE")]
private class Record
{
}

var database = await magic.OpenAsync(new DbStore()
{
    Name = "SingleRecordBasic.Add",
    Version = 1,
    StoreSchemas = [SchemaHelper.GetStoreSchema(typeof(Record))]
});

I would suggest to support null for database name, which could be used by GetStoreSchema, but won't be included in GetAllSchemas.

(Our unit tests need to share a type across multiple databases; Otherwise we have to write a same type repeatedly.)

[magiccodingman]

Btw I'm about to Obsolete the "Execute" method and introduce:

Method	Description
ToListAsync()	Executes the MagicQuery and returns the results as List<T>.
AsEnumerableAsync()	Executes the MagicQuery and returns the results as IEnumerable<T>.
ToList()	Executes the MagicQuery and returns the results as List<T>.
AsEnumerableAsync()	Executes the MagicQuery and returns the results as IEnumerable<T>.

Anyways, let me fix the magic table name not being respected in the serializer. That's for sure a serializer issue. I think I forgot to implement that logic lol.

Also on the other things you brought up, I will get back to you on that. It's getting late for me so I'm hopping off soon. But we'll want unit tests btw for the new methods being introduced as well that I just merged to Main. But lots of new capabilities have been built in today. I had a lot of fun refactoring things. Should I have done other stuff? Yes, but hey, I'ma do what I do I guess. ttyl

[magiccodingman]

@yueyinqiu Right before I fell asleep I thought of something haha. When you said the schema name isn't respected by the magic table attribute. Was that a test after my most recent main pushes? Or prior? Just was wondering because something I did may have changed that and fixed it accidentally. Check it out again after pulling the most recent main changes. If it doesn't work still, I'll look into it in the morning

[yueyinqiu]

I think both? But this is the behavior I expect.

[yueyinqiu]

By the way, it seems that the current naming policy is switched back to:

Is this intentional? (So I should change the expected value in unit tests.)

[magiccodingman]

Uhhm, yea I think that was part of a change I made where I realized something was wrong. Basically the new rule policy is that by default things tend to be camel case, but if anything has a magic attribute it's never camel case, as it must match your code. Even if you have a MagicTable attribute, it makes it so all properties are never camel cased. As those should match what you've specified.

[magiccodingman]

I think both? But this is the behavior I expect.

Expected? Also I just tested it. I did repair it in the most recent update. The parameter on MagicTable called, "schemaName" is now respected

[yueyinqiu]

Oh, I mean database name, not schema (store) name.

[magiccodingman]

Oh gotcha, I'll check that out

[magiccodingman]

@yueyinqiu So I think I accidentally left behind that logic. Which is why it's not working. Just something I forgot to keep track of I think. I'll make a separate PR, but I believe that database name in the attribute isn't respected in a lot of places. Just needs to be updated to do so. Shouldn't be very complicated.

[magiccodingman]

Oh I see the issue already. I need to figure out why I coded it the way that I did. Because within, "public class DbStore" there's, "public string Name { get; set; }" which is the name of your DB. But why is that there? The attribute should inform the code as to what db it's in so that you can pick and choose.

What's the point of that Name variable? Because everything is using it right now, but I think that may have been an oversight on my end.

[magiccodingman]

Wait I kind of see why I did this. You're supposed to get the registered Async by the DbName and that way you can have multiple registered. But I think that's kind of unnecessary potentially. Because I should have all that information already from the attributes. I could make just a single service that rules them all, no more passing in db names or anything. It can just know. This would require some refactoring on any current system utilizing Magic.IndexDb, but I could make this simpler. This would remove the "Name" property in DbStore as well and then you'd get the database name from cache in the SchemaHelper which would be done internally.

[magiccodingman]

Oh man there's a couple things I did that doesn't make sense. I don't know why I made the db name so complicated. But then I kind of do at the same time? But I am kind of just starting to feel like the idea of not worrying about the dbName and just working off the attributes is best. But I think this is legacy logic from when I forked off of BlazorDB by newestfall. In which very very little of that code is left in this project. So the project is nearly unrecognizable from where we started. Though I wouldn't be surprised if I coded that or if that's legacy from BlazorDB.

UPDATE

I just looked out of curiosity. And I now understand why this exists. Look at Nwestfalls original code of DbStore:

public class DbStore
    {
        public string Name { get; set; }
        public int Version { get; set; }
        public List<StoreSchema> StoreSchemas { get; set; }
    }

It's identical to what is in Magic IndexDB. And I'm now remembering why I branched off of Nwestfalls long abandoned project versus more recent ones because I felt like it was much more down the right direction. But the BlazorDb didn't work off of this attribute system I created. And it was mostly about working with much smaller data sets and a lot more manually written items with dynamic objects that required constant runtime reflections.

This is legacy logic. I think I've confirmed not only the roots of where this originates from, but that it's also logic that's unnecessary in our project.

Actually now after review, I don't think it's necessary at all either to have the "StoreSchemas". I have all that knowledge, it's unnecessary for the user to be required to set.

The only reason at all that you'd need to set your own custom store schema is if you wanted to do this at startup or add things dynamically like so:

options.StoreSchemas = new List<StoreSchema>()
    {
        new StoreSchema()
        {
            Name = "Person",
            PrimaryKey = "id",
            PrimaryKeyAuto = true,
            UniqueIndexes = new List<string> { "name" }
        }
    };

Which kind of fits the idea you were talking about @yueyinqiu with a more dynamic system.

I think it's very fair to decide moving forward what we accept, what we make users declare, and how the library works moving forward. I think Magic.IndexDB has diverged much too drastically from the initial fork of BlazorDB and it's time to decide the best path.

Conclusion

There may be a point to the DbName and calling the GetRegistered or OpenAsync per DB. Because now that I think about it. You could argue that you don't want to open db's willy nilly without declaration.

But I could also argue that I could open tables dynamically when necessary and leave open after a call, but then you don't have the ability to close databases either manually.

So, do we want to continue manually calling which DB tables you do or don't open? I personally am leaning towards no more manually calling.

And do we keep the store shema logic on startup since that's something I can handle without user intervention?

Note I do think users should be able to more easily choose when to open or close databases, but more specifically how? I think there should be some sort of concept that allows users to decide a bit differently than what is being done now. Because personally, I don't want to juggle multiple instances of manager. Nor do you know easily in runtime what model goes to what DB. And it shouldn't matter because we track all of that.

The question is how do we conceptualize this?

[magiccodingman]

Hmmm @yueyinqiu I'm actually considering a pretty substantial refactor #60