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

Support IMutableNavigation.SetJsonPropertyName/IReadOnlyNavigation.GetJsonPropertyName - CosmosDB Provider #27328

Closed
georgechond94 opened this issue Feb 1, 2022 · 5 comments
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported

Comments

@georgechond94
Copy link

georgechond94 commented Feb 1, 2022

I am setting the json property names for all the non-shadow properties in my model as such:

var scalarProperties = builder.Model.GetEntityTypes()
                .SelectMany(et => et.GetProperties().Where(p => !p.IsShadowProperty()).ToList());

foreach (var mutableProperty in scalarProperties)
{
    var str = mutableProperty.GetJsonPropertyName();
    mutableProperty.SetJsonPropertyName(char.ToLowerInvariant(str[0]) + str[1..]);
}

The above works well for scalar properties, although the same functionality is not there for navigation properties:

var navigationProperties = builder.Model.GetEntityTypes()
                .SelectMany(et => et.GetNavigations().Where(p => !p.IsShadowProperty()).ToList());

foreach (var mutableNavigation in navigationProperties)
{
    var str = mutableNavigation.GetJsonPropertyName(); // doesn't exist
    mutableNavigation.SetJsonPropertyName(char.ToLowerInvariant(str[0]) + str[1..]); // doesn't exist
}

I tried setting the CosmosAnnotationNames.PropertyName annotation explicitly but it seems that it's being ignored for the IMutableNavigation objects.

public static void SetJsonPropertyName(IMutableNavigation navigation, string? name)
    => navigation.SetOrRemoveAnnotation(CosmosAnnotationNames.PropertyName, name);

Any reason why this is not supported?

Thanks!

@georgechond94
Copy link
Author

georgechond94 commented Feb 1, 2022

Here is how a document looks like:

{
    "id": "Idf40a006c-f8e8-47e7-804f-3ca9bc890d91",
// scalar properties are named correctly.
    "name": "Name1aa46385-77a1-4a16-ac6b-b4d07eefe02d",
    "_etag": "\"00000000-0000-0000-1774-f210fcf501d8\"",
// navigation properties still have the default names
    "Slot": {
        "capacity": 10,
        "Products": [
            {
                "productTypeId": "ProductTypeIdc571369b-89ba-443a-a7e2-686cbf5021c0",
                "productVolume": 1
            }
        ]
    },
    "_rid": "XjEOAJc-lqsGAAAAAAAAAA==",
    "_self": "dbs/XjEOAA==/colls/XjEOAJc-lqs=/docs/XjEOAJc-lqsGAAAAAAAAAA==/",
    "_attachments": "attachments/",
    "_ts": 1643724405
}

@georgechond94 georgechond94 changed the title Support IMutableNavigation.SetJsonPropertyName/IMutableNavigation.GetJsonPropertyName - CosmosDB Provider Support IMutableNavigation.SetJsonPropertyName/IReadOnlyNavigation.GetJsonPropertyName - CosmosDB Provider Feb 1, 2022
@AndriySvyryd
Copy link
Member

AndriySvyryd commented Feb 2, 2022

Try navigation.TargetEntityType.SetContainingPropertyName()

@AndriySvyryd AndriySvyryd added the closed-no-further-action The issue is closed and no further action is planned. label Feb 2, 2022
@AndriySvyryd AndriySvyryd removed their assignment Feb 2, 2022
@georgechond94
Copy link
Author

Hi @AndriySvyryd, thanks for your reply, that works as expected!

Question: do you think it makes sense to align that part a bit so these extension methods exist for both INavigation & IProperty?

I tried to implement that as my first contribution to efcore here: georgechond94@719ebf0
It works as expected and there are no failing tests.

Let me know if that change is welcome, if yes, I will go on with creating some tests & a PR.

@AndriySvyryd
Copy link
Member

AndriySvyryd commented Feb 3, 2022

I don't think it improves the experience that much. Hopefully #15258 will make it unnecessary to use the methods on IMutable* interfaces

@georgechond94
Copy link
Author

clear, thanks!

@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported
Projects
None yet
Development

No branches or pull requests

3 participants