Breeze / breeze-client

Breeze for JavaScript clients
MIT License
38 stars 16 forks source link

Cannot read property 'clientPropertyPathToServer' of undefined #47

Closed graphicsxp closed 2 years ago

graphicsxp commented 3 years ago

I'm encountering the following error.

image

This happens when I build a query with the following predicate :

return new Predicate(JSON.parse( JSON.stringify(this.whereClause))); where this.whereClause is the following JSON { publications: { any: { title: { contains: 'some value' } } } }

This is something that I've had working in the past in previous projects but for some reason breeze-client is throwing that error now. It crashes in predicate.ts. entityType is undefined for the second part of the query (propertyPath = 'title'). I'm not even sure where toNameOnServer is set but it is always true.

 propExpr: function (this: PropExpr, context: VisitContext) {
    if (context.toNameOnServer) {
      return context.entityType!.clientPropertyPathToServer(this.propertyPath); 
    } else {
      return this.propertyPath;
    }
  },

The metadata looks like this for the queried object (class Request) image and for the publications navigationProperty the metadata for class Publication look like this :
image

Any idea where I could look at to solve this ?

steveschmitt commented 3 years ago

I noticed that you JSON.parse( JSON.stringify(this.whereClause)) when building the Predicate. Do you get the same error if you don't stringify and parse? Is the whereClause changed by this?

Also, when you said

entityType is undefined for the second part of the query (propertyPath = 'title').

What do you mean by "the second part"? Do you mean that the first part of the query is the "any" and the second part is the "contains title"?

Thanks for reporting this, and thanks for helping me understand.

graphicsxp commented 3 years ago

The propExpr function is hit twice. The first time with this.propertyPath = 'publications' (that's the first part of the where clause indeed) and the second time with this.propertyPath = 'title' and that's when it crashed because entityType is undefined.

The stringify is just because i do a find and replace in the JSON (not pasted here because it is irrelevant to the problem) Nevertheless I did try without the stringify and the result is the same.

Here's a screenshot of what the predicate looks like in the end image

Anything that seems suspicious ? Do you know why that enittyType property would be undefined ? I'm not sure i understand what context.toNameOnServer does ?

steveschmitt commented 3 years ago

I think context.toNameOnServer is a flag to indicate that the query should be expressed in terms of the server's names for the entity properties. For example, some servers have property names in PascalCase, but they are camelCase on the client. The clientPropertyPathToServer function would convert those names based on the NamingConvention in use.

I'll see if I can duplicate the problem.

graphicsxp commented 3 years ago

ah yes you are correct about the flag. My properties are indeed converted from camelCase back to PascalCase when querying the server, which is the expected behavior. It works absolutely fine as long as I don't use they "any" operator to query on properties of navigation property.

Some more info on what is happening Just before propExpr is hit, we are in the function anyAllPredicate line 1330.

image

Here context.entityType is defined and it goes on to visit and to propExpr, which works fine for the first part of the any

Then a newContext is created and its entityType is set from this.expr.dataType, which is actually undefined . That's how we end up with the undefined èntityTypein the next call topropExpr` for 'title' in my case.

The question remains. Why is it undefined...

graphicsxp commented 3 years ago

last bit of information, but not the least. I have debug another project where I'm using the exact same scenario and for which I do not have this issue. It is working because toNameOnServer is false. Therefore it fallbacks to using this.propertyPath.... and in that working project I had manually specified the property name as Pascalcase because that's how the server expects it.

So, my question is how do I set toNameOnServer to false ? Because I never did that manually in my past project. How come it is set to true in my current project ? Could it be related to the fact I'm using .net core to generate metadata ? (In working project I was using .net framework 4.6)

graphicsxp commented 3 years ago

I found the problem.

In my past project I used UriBuilderODataAdapter.register();

Now I have to use UriBuilderJsonAdapter.register();

And this adapter hardcodes toNameOnServer to true !

image

So how can I solve this ?

Also I would not be surprised if the issue I reported 2 days ago is also related to this adpater, because I never had this problem with the OdataAdapter : https://github.com/Breeze/breeze-client/issues/46

EDIT Alternatively I'd be happy to use the ODataAdapter if you could help with breeze.net core as this is the error I get when I use this adapter :

System.Exception: This EntityQuery ctor requires a valid json string. The following is not json: $filter=Id eq guid'3335ddba-1126-46ab-82d7-ae06abb60000'
   at Breeze.Core.EntityQuery..ctor(String json)
   at Breeze.AspNetCore.BreezeQueryFilterAttribute.OnActionExecuted(ActionExecutedContext context)

I don't have this issue with breeze .net

steveschmitt commented 3 years ago

I don't know why the UriBuilderJsonAdapter hardcodes toNameOnServer. I'll look into that.

We don't support OData on .NET Core yet, but it's on our radar.

graphicsxp commented 3 years ago

hello Steve Have you had a chance to check the Jsonadapter code and see why it does not behave the same way as the odatAdapter ? This is a rather major issue for us, would be nice to have this fixed :)

Also do you have a rough idea when .net core version would support odata ?

steveschmitt commented 3 years ago

@graphicsxp I haven't been able to reproduce the problem. I've tried with our usual NorthwindIB test model, using Customers and Orders.

  test("Any predicate constructor - issue #47", function (assert) {
    // https://github.com/Breeze/breeze-client/issues/47
    var done = assert.async();

    var em = newEm();
    var j1 = { orders: { any: { shipName: { contains: 'maison' } } } };
    var p1 = new Predicate(j1);
    var q1 = EntityQuery.from("Customers").where(p1);

    var s = q1.wherePredicate.toString();
    ok(s == '{"orders":{"any":{"shipName":{"contains":"maison"}}}}', s);

    var queryUrl = q1._toUri(em);
    ok(queryUrl == 'Customers?%7B%22where%22%3A%7B%22Orders%22%3A%7B%22any%22%3A%7B%22ShipName%22%3A%7B%22contains%22%3A%22maison%22%7D%7D%7D%7D%7D', queryUrl);

    em.executeQuery(q1).then(function (data) {
      custs = data.results;
      var len = custs.length;
      ok(len == 2, "Should have 2 records: " + len);

    }).fail(testFns.handleFail).fin(done);
  });

In my tests, whenever it reaches the propExpr function in Breeze, context.entityType is always defined if context.toNameOnServer is defined.

I don't know what is different about your queries or your metadata.

BTW, I'm using breeze-client 2.0.11 and Breeze.Persistence.EFCore 5.0.4

graphicsxp commented 3 years ago

I have investigated further and I found something very odd.

If the name of my endpoint matches the name of the entity (in your case that'd be "Customers"), then my "any" filter works just fine ! And it works whether I use the JSON format in my where clause or just the regular api :

query = EntityQuery.from('Requests')      
             .where({ 'publications': { 'any': { 'title': { 'contains': 'something' } } } }) 

query = EntityQuery.from('Requests')      
             .where('publications', FilterQueryOp.Any, 'title', FilterQueryOp.Contains, 'something')

However if I change the name of the endpoint to something else :

query = EntityQuery.from('RenamedEndpoint')      
             .where('publications', FilterQueryOp.Any, 'title', FilterQueryOp.Contains, 'something')

Then I get the error we discussed above when the where predicate is being built.

This is very strange because having the endpoint name not matching the entity name has always worked before. It works with any other kind of filters, orderby, etc...

Could you try with your unit test and rename Customers to something else ?

I also use breeze-client 2.0.11 with Breeze.Persisrtence.EFCore 3.1.4

steveschmitt commented 3 years ago

You are right! When I change the endpoint to "CustomersAgain":

var q1 = EntityQuery.from("CustomersAgain").where(p1);

then I get the "clientPropertyPathToServer" error.

The problem is that the EntityType is unknown at this point, because there's no mapping from the "CustomersAgain" endpoint to the "Customer" EntityType, so Breeze doesn't know how to convert the path.

We could make a fallback to just use the property name as-is, but the right way to fix it is to tell Breeze the type using toType:

var q1 = EntityQuery.from("CustomersAgain").toType("Customer").where(p1);

This fixes the problem, and the query is executed correctly. toType may be the fix for #40 also.

If you don't want to add toType everywhere in your code, you can tell the MetadataStore the correct EntityType for the endpoint:

entityManager.metadataStore.setEntityTypeForResourceName("CustomersAgain", "Customer");

You should do this when the app is starting up, before creating any queries.

graphicsxp commented 3 years ago

So this is the problem indeed. If I use toType , it solves both this issue and #40.

The bad news is that in return my data is converted to a breeze entity of the type that I specify in toType. I do not want that and this is specifc to my use-case which is the following :

I want to use breeze/odata to query my data from the server. However on the server-side, inside the breeze endpoint, the IQueryable is altered and instead of returning full-blown entities, I return a light object, kind of like a DTO.

On the client-side, these list of DTO is displayed on the screen to the user, but I want it to remain as light as possible. I don't want this data to be breeze entities.

Is it possible to use toType in order to fix all these issues that I'm facing, but at the same time having the data returned from the server staying as such and not converted by breeze to the full-blown entity ?

steveschmitt commented 3 years ago

You could use .select() on the client side so that Breeze knows it is getting a projection instead of entities. But I don't know how toType() interacts with select(); you might end up with the same problem.

You could register your DTO as an entity type in Breeze metadata, so that Breeze knows about the properties on it. But then you would still end up with entities; they would be your DTO entities.

I think the best answer is to use .toType() and also .noTracking(). The .noTracking() option tells Breeze not to make entities from your query results.

BTW, here is the doc about resource names / endpoints.

graphicsxp commented 3 years ago

Thanks fo this tip. We're almost there ! The noTracking method sounds great, but according to the documentation :

_However, the following “entity” services are still performed

graph cycle resolution property renaming datatype coercion_

My DTO has some property names that do not match the ones of the full breeze entity. So unfortunately when I use noTracking(), the data is a simple javascript object but with some properties that don't match what is returned by the server.

It would be great if this could be parameterized in the noTracking function. A simple flag renameProperties would do.

Do you think this could be added to breeze-client ?

steveschmitt commented 3 years ago

The renameProperties flag wouldn't be that simple, because it would affect several layers of code.

The best thing may be for you to register your DTO type as an entity type in the metadata. Then you could control the property names for querying and result processing.

steveschmitt commented 3 years ago

Regarding the original error:

For the next release, I'm going to change the Predicate code so that instead of throwing an error, it will be a warning that advises you to use toType. Since it is only a warning, the query can still be converted to a URL, but the property paths will not be mapped, so you may get errors from the server.

steveschmitt commented 3 years ago

@graphicsxp In release 2.1.0, I've changed the code so the Predicate code writes a warning, instead of throwing an error.

Please try it and let me know if that helps your issue.

ASA018 commented 2 years ago

Hello Steve,

I'm using .Net Core 6 with Breeze and Angular 12, please what is the equivalent of Odata for .Net Core 6? Breeze nuget packages

Below the used versions of breeze-bridge2-angular and breeze-client on package.json file: "breeze-bridge2-angular": "^1.1.0", "breeze-client": "^1.6.3",

Regards, Amal,

steveschmitt commented 2 years ago

@ASA018 You don't need Odata with newer versions of Breeze. Take a look at the Northwind Demo application to see how to use Breeze on client and server.