fix: sparse fields set not applied to query

[maurei]

continuation of #476

Indeed, @milosloub, there are still some issues; I was too hasty with replacing Get with the new GetQueryable.

Interestingly enough tests are not failing. Some investigation about this:

• Fields_Query_Selects_Sparse_Field_Sets is not failing because, even though the sparse field set is not processed in the SQL query, the DocumentBuilder still removes the fields from the output JSON. This behaviour annoys me: (de)serialization should be decoupled from JsonApiContext, see Deserializer only links attributes and not included relationships #497. I don't think we can properly integration test the usage of the new GetQueryable without getting rid of this behaviour in the serialization layer, but that is beyond the scope of this PR.

• I suspect the same goes for the other sparse field set tests.

I have changed a few things that should apply it to the queries again. All tests are passing, let me know if you have any more comments or an idea how we could more thoroughly test the new GetQueryable. (Note that the previous Get wasn't properly tested for sparse field selection neither)

[milosloub]

Yeah, you are right. I totally forgot about that logic in DocumentBuilder. I remember, that there is a problem with this - for example you have integer property, so default value is 0 and you are not able to distinguish if it is standard value stored in DB, or if it is default value while new instance is created and you are not requesting this property (using fields without this property). Right now I don't have idea how to solve this other than this approach :/

[maurei]

... so default value is 0 and you are not able to distinguish if it is standard value stored in DB, ...

You're right, I don't think it's possible like this. Even if we only supported sparse fields selection for nullable fields to distinguish between 0 and default(int) (eg nullable int), it still wouldn't cover every scenario because there might be cases in which users actually want to use null as meaningful data. The same goes for strings and most/all other datatypes.

So yep, the (de)serialization layer, when sparse field selection is involved, can not completely (although still more than it is now) be decouple from JsonApiContext .

With respect to this I have the following thoughts:

• We can still better organise the deserializers so that they become more reusable. Eg: if the deserializer would be used in a JsonApiClient, it will never have to worry about sparse fields. So I think that DocumentBuilder still should naively convert entities into documents without relying on JsonApiContext, and the actual IJsonApiDeserializer can perform additional logic, like

  • taking into account sparse fields when the serializer is used in a server (public class IJsonApiServerDeserializer : IJsonApiDeserializer`)
  • or removing metadata / links when used in a client (public class IJsonApiClientDeserializer : IJsonApiDeserializer`)

• Sparse field selection on the level of the repositories should be tested using integration tests that involve only repositories, using an in-memory database. The current Fields_Query_Selects_Sparse_Field_Sets test is actually an e2e test.

Anyway, this is both ofc beyond the scope of this PR

[milosloub]

This looks good to me. I think we can go like this with no problem.