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

CSharpHelper: support more types of collection literals #19274

Closed
AdrianoAE-Surface opened this issue Dec 11, 2019 · 8 comments · Fixed by #28212 or #28330
Closed

CSharpHelper: support more types of collection literals #19274

AdrianoAE-Surface opened this issue Dec 11, 2019 · 8 comments · Fixed by #28212 or #28330
Assignees
Labels
area-tools closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. community-contribution customer-reported type-enhancement
Milestone

Comments

@AdrianoAE-Surface
Copy link

AdrianoAE-Surface commented Dec 11, 2019

I've been hacking around trying to extend the modelbuilder and I came across this CSharpHelper and need some clarifications.
Class:

public class ConfigTest
{
    public Type Type { get; set; }
    public string ForeignKeyName { get; set; }
}

And i'm adding a annotation to the EntityTypeBuilder

IEnumerable<ConfigTest> config = new List<ConfigTest>() { ... }
builder.Metadata.AddAnnotation("ConfigTestKey", config);

But then when creating the migration the following error is thrown

The current CSharpHelper cannot scaffold literals of type 'System.Collections.Generic.List`1[ConfigTest]'. Configure your services to use one that can.

So after digging around I got to the CSharpHelper and the UnknownLiteral. After messing around trying to understand what I had to do I ended up doing this for no reason and it worked

public override string UnknownLiteral(object value)
{
    if (value is IEnumerable<ConfigTest> configuration)
    {
        var test = configuration.Select(x => "").ToArray();
        return Literal(test);
    }

    return base.UnknownLiteral(value);
}

After that and injecting my custom CSharpHelper everything works and I can use my List as value of the annotation. But how does it work? Why passing a empty array of strings with the size of my List works and the actual value can be casted back to the List when building the model. What is going on under the hood?

I couldn't find mutch information about the CSharpHelper or even how to implement the UnkownLiteral, I just got it working by accident and I don't understand why it works.

@AdrianoAE-Surface AdrianoAE-Surface changed the title [Clarification Wanted] CSharpHelper - UnknownLiteral [Clarification] CSharpHelper - UnknownLiteral Dec 11, 2019
@ajcvickers
Copy link
Member

@AdrianoAE-MA It would probably help here if you could describe your goal in some more details. What is the reason for the annotation and how do you intend to use it?

We will consider updating the C# helper to support generating literals from IEnumerables.

@AdrianoAE
Copy link

AdrianoAE commented Dec 15, 2019

I'm trying to learn some of the inner working of the entity framework so I can build some extensions. In this particular case I ended up just refactoring the code to use a static configuration class. During the process I just ended up in that class and while trying to understand it I got to that solution that (for the knowledge I have) makes no sense how the value goes to the actual annotation while I'm just passing to the CSharpHelper a array of empty strings.

In terms of what I'm trying to do, I'm trying to automate localization for my domain but I don't want my domain to have any knowledge of it since it's not it's responsability. Giving a concrete example:
Domain Class:

public class Product
{
    public int Id { get; private set; }
    public string Name { get; private set; }
    public string Description { get; private set; }
    public decimal Price { get; private set; }
    //...
}

In my persistence I've created some EntityTypeBuilder extension where I can configure like that

internal class ProductConfigurations : IEntityTypeConfiguration<Product>
{
    public void Configure(EntityTypeBuilder<Product> productConfiguration)
    {
        productConfiguration.PropertyWithTranslation(p => p.Name)
            .HasMaxLength(100)
            .IsRequired();

        productConfiguration.PropertyWithTranslation(p => p.Description)
            .HasMaxLength(400)
            .IsRequired();
    }
}

And since I don't have access to the LanguageEntity (because it's a completely different project) I had to allow some way to configure a "ShadowTable" where I had to say what are the primary Keys (that's where that problem emerged. I was trying to use a Annotation in the model builder to get those values into my extension.

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    modelBuilder.ApplyConfigurationsFromAssembly(typeof(T).Assembly);
    modelBuilder.ApplyTranslationsConfigurations(new List<PrimaryKeyConfiguration>
        {
            new PrimaryKeyConfiguration(typeof(int), "LanguageId"),
            new PrimaryKeyConfiguration(typeof(Guid), "UserId")
        });
}

Repository of the actual project: https://github.com/AdrianoAE/AdrianoAE.EntityFrameworkCore.Translations

Now I'm stuck trying to understand how I can allow my entity to be used normally and on save changes / on querying use the selected language key and get my domain object. No luck so far 😕

@ajcvickers
Copy link
Member

@AdrianoAE-MA That's a fairly ambiguous goal. I'm not sure this will be possible without significant hacking.

With regards to the original question, we are putting this on the backlog to support more collection types. For now you should be able to just use an array instead of a List.

@ajcvickers ajcvickers added this to the Backlog milestone Dec 15, 2019
@ajcvickers ajcvickers changed the title [Clarification] CSharpHelper - UnknownLiteral CSharpHelper: support more types of collection literals Dec 15, 2019
@AdrianoAE-Surface
Copy link
Author

@ajcvickers Regarding the CSharpHelper what I don't understand is why a empty array of string works.
That was my Class:

public class PrimaryKeyConfiguration
{
    public readonly Type Type;
    public readonly string ForeignKeyName;

    public PrimaryKeyConfiguration(Type type, string foreignKeyName)
    {
        Type = type;
        ForeignKeyName = !string.IsNullOrWhiteSpace(foreignKeyName) ? foreignKeyName : throw new ArgumentNullException(nameof(ForeignKeyName));
    }
}

I was passing to the Annotation

var keys = new List<PrimaryKeyConfiguration> { ... }
builder.Metadata.AddAnnotation("keyConfig", keys).

And then during the OnModelCreating I was casting that annotation back to IEnumerable.

That's when I got the error in the CSharpHelper but this works.
return Literal(configuration.Select(x => "").ToArray());
That's what I don't understand, how does it work when I'm just creating a array of empty string from my collection and the values are successfully casted back to the actual List.


In terms of the actual goal, I know that it will require quite a lot of work and probably won't be able to do the way I was originally thinking. But at least It's being a good project to help learn some of the internals of EFCore even tho in a very basic level.

@ajcvickers
Copy link
Member

Regarding the CSharpHelper what I don't understand is why a empty array of string works.

Because the code only handles arrays, not Lists.

And then during the OnModelCreating I was casting that annotation back to IEnumerable.

It's not casting to IEnumerable. List implements IEnumerable. So List can be used any place an IEnumerable is expected.

yinzara added a commit to yinzara/efcore that referenced this issue Jun 13, 2022
@yinzara
Copy link
Contributor

yinzara commented Jun 13, 2022

I'm going to implement List literals in CSharpHelper and support for ListInit Expressions which should make this work as well as add support needed by:
npgsql/efcore.pg#2402

yinzara added a commit to yinzara/efcore that referenced this issue Jun 13, 2022
yinzara added a commit to yinzara/efcore that referenced this issue Jun 13, 2022
yinzara added a commit to yinzara/efcore that referenced this issue Jun 13, 2022
yinzara added a commit to yinzara/efcore that referenced this issue Jun 13, 2022
@ghost ghost closed this as completed in #28212 Jun 27, 2022
ghost pushed a commit that referenced this issue Jun 27, 2022
* Add support for CSharpHelper for List literals

Fixes #19274

Also relates to npgsql/efcore.pg#2402

* Fixes from review comments
@roji roji modified the milestones: Backlog, 7.0.0 Jun 27, 2022
@roji roji added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jun 27, 2022
@roji
Copy link
Member

roji commented Jun 27, 2022

Leaving open to support yet other collection literal types; Dictionary specifically could be useful for JSON scenarios.

@roji roji reopened this Jun 27, 2022
@roji roji modified the milestones: 7.0.0, Backlog Jun 27, 2022
@roji roji removed the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jun 27, 2022
@yinzara
Copy link
Contributor

yinzara commented Jun 27, 2022

If I was to suddenly implement Dictionary literals, would you prefer the pre .NET 6 syntax using collection initializers i.e.:

new Dictionary<string, string>
{
   { "key1", "value1" },
   ...
}

or the .NET 6 indexed based variety:

new Dictionary<string, string>
{
   ["key1"] = "value1",
   ...
}

I know this would go into EF for .NET 7 but I didn't know if we prefered to be backward compatible.

Also should we put a space after the , in new Dictionary<string, string>() or not new Dictionary<string,string>()

yinzara added a commit to yinzara/efcore that referenced this issue Jun 27, 2022
yinzara added a commit to yinzara/efcore that referenced this issue Jun 27, 2022
yinzara added a commit to yinzara/efcore that referenced this issue Jun 28, 2022
yinzara added a commit to yinzara/efcore that referenced this issue Jun 29, 2022
yinzara added a commit to yinzara/efcore that referenced this issue Jul 9, 2022
@bricelam bricelam self-assigned this Aug 16, 2022
@bricelam bricelam added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Aug 16, 2022
@bricelam bricelam modified the milestones: Backlog, 7.0.0-rc2 Aug 16, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.0-rc2, 7.0.0 Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-tools closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. community-contribution customer-reported type-enhancement
Projects
None yet
6 participants