ArangoDB-Community / arangodb-net-standard

A consistent, comprehensive, minimal interface to enable .NET applications to use the complete range of features exposed by the ArangoDB REST API.
Apache License 2.0
76 stars 30 forks source link

PostInvertedIndexBody Serialization #470

Closed kealist closed 1 year ago

kealist commented 1 year ago

Just documenting though I think this has been fixed in master, but not a release. There are serialization errors when running the following code. I assume this was something from PostInvertedIndexBody overloading Fields property.


Message: 
ArangoDBNetStandard.Serialization.SerializationException : An error occured while Deserializing the data response from Arango. See InnerException for more details.
---- Newtonsoft.Json.JsonReaderException : Unexpected character encountered while parsing value: {. Path 'fields', line 1, position 292.

  Stack Trace: 
ApiClientBase.DeserializeJsonFromStream[T](Stream stream)
IndexApiClient.PostIndexAsync(PostIndexQuery query, PostIndexBody body, CancellationToken token)
IndexApiClient.PostInvertedIndexAsync(PostIndexQuery query, PostInvertedIndexBody body, CancellationToken token)

var collection = new PostIndexQuery { CollectionName = "document" };
var postBody = new PostInvertedIndexBody
{
    Name = "inverted",
    Fields = new List<InvertedIndexField> { new InvertedIndexField { Name = "description", Analyzer = "text_en" } }
};
await db.Index.PostInvertedIndexAsync(collection, postBody);

This has just been a hassle with the index changes in 3.10 and trying to follow the index guide, which don't work with this client.

DiscoPYF commented 1 year ago

Thanks for reporting this issue @kealist

Just documenting though I think this has been fixed in master, but not a release.

You're right, it was fixed by https://github.com/ArangoDB-Community/arangodb-net-standard/issues/443 which is in master but hasn't been released yet.

Your idea of releasing pre-release packages (or even daily builds packages) is a good one: https://github.com/ArangoDB-Community/arangodb-net-standard/issues/468

@tjoubert I noticed you started merging some of the breaking change PRs into master for the next major release. We could create a pre-release package, e.g. 2.0.0-alpha.1, that will contain the fix.

As per https://learn.microsoft.com/en-us/nuget/create-packages/prerelease-packages#semantic-versioning

-alpha: Alpha release, typically used for work-in-progress and experimentation -beta: Beta release, typically one that is feature complete for the next planned release, but may contain known bugs. -rc: Release candidate, typically a release that's potentially final (stable) unless significant bugs emerge.

The steps would be:

  1. Create a commit and PR to update the csproj version to 2.0.0
  2. Create a pre-release in github which will tag the pre-release commit
  3. Build a nuget package from that commit

I'm happy to do that. Not sure we need step 2 though, but probably good for clarity?

@kealist Would it be useful for you?

kealist commented 1 year ago

@DiscoPYF Yes anything would be great in the short term for me so I don't have to work around things. I don't know the ideal way to do that with how releases/prereleases are done on Github, I've only done it internally at my company using gitlab CI + gitversion.

DiscoPYF commented 1 year ago

Yes anything would be great in the short term for me so I don't have to work around things.

Ok, I'll confirm with @tjoubert before I create the pre-release.

Alternatively, you can build a pre-release package yourself, but I don't know if your company policy/infra allows. Steps:

That may be a good compromise while we get something to you.

DiscoPYF commented 1 year ago

@tjoubert There is also the idea of releasing this fix (https://github.com/ArangoDB-Community/arangodb-net-standard/issues/443) into a patch release? e.g. 1.3.1

While the fix contains a breaking change, it fixes an endpoint that's broken anyway with ArangoDB 3.10 so that may be acceptable. Not sure if it's a good practice, as it will be a breaking change for anyone using the feature with ArangoDB 3.9 .

DiscoPYF commented 1 year ago

We could also simply create multiple major releases in a row, instead of a pre-release.

For example, instead of releasing 2.0.0-alpha.1 and later 2.0.0, release 2.0.0 with what's on master now and later release a 3.0.0 with the additional breaking changes scheduled.

That's probably the simplest right now 🤔

tjoubert commented 1 year ago

Hi @DiscoPYF. Yes, I think a 2.0.0 now and a 3.0.0 later would be the simplest. See if there is any of the other PRs that you can quickly review, approve and merge for v2.0.0. Thanks!

DiscoPYF commented 1 year ago

Thanks for confirming @tjoubert . I've merged what I could for now. I will have more time next week to review the remaining pull requests.

Merged:

https://github.com/ArangoDB-Community/arangodb-net-standard/pull/446 https://github.com/ArangoDB-Community/arangodb-net-standard/pull/438 https://github.com/ArangoDB-Community/arangodb-net-standard/pull/458

I will prepare the 2.0.0 release.

DiscoPYF commented 1 year ago

I've just realized that it would have been best to keep the amount of changes in 2.0.0 to the minimum, so that people affected by the bug can update more seemlessly. 🤔

DiscoPYF commented 1 year ago

@kealist The release is out: https://github.com/ArangoDB-Community/arangodb-net-standard/releases/tag/2.0.0

Let us know how the update goes and whether you are able to call PostInvertedIndexAsync without issues.

kealist commented 1 year ago

@DiscoPYF It's currently throwing an exception on deserializing the response (JSON integer 5368709120 too large for int32), but I'll try to get more info

  Message: 
ArangoDBNetStandard.Serialization.SerializationException : An error occured while Deserializing the data response from Arango. See InnerException for more details.
---- Newtonsoft.Json.JsonReaderException : JSON integer 5368709120 is too large or small for an Int32. Path 'consolidationPolicy.segmentsBytesMax', line 1, position 201.

  Stack Trace: 
ApiClientBase.DeserializeJsonFromStreamAsync[T](Stream stream)
IndexApiClient.PostInvertedIndexAsync(PostIndexQuery query, PostInvertedIndexBody body, CancellationToken token)
ArangoDbGraphGenerator.CreateGraphSetupAsync() line 362
ArangoDbGraphGenerator.InitializeOntologyGraphAsync() line 88
ArangoDbGraphTests.ArangoDatabaseAndGraphGenerationTests() line 57
--- End of stack trace from previous location ---
----- Inner Stack Trace -----
JsonTextReader.ParseReadNumber(ReadType readType, Char firstChar, Int32 initialPosition)
JsonTextReader.ParseNumber(ReadType readType)
JsonTextReader.ReadNumberValue(ReadType readType)
JsonTextReader.ReadAsInt32()
JsonReader.ReadForType(JsonContract contract, Boolean hasConverter)
JsonSerializerInternalReader.PopulateObject(Object newObject, JsonReader reader, JsonObjectContract contract, JsonProperty member, String id)
JsonSerializerInternalReader.CreateObject(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
JsonSerializerInternalReader.SetPropertyValue(JsonProperty property, JsonConverter propertyConverter, JsonContainerContract containerContract, JsonProperty containerProperty, JsonReader reader, Object target)
JsonSerializerInternalReader.PopulateObject(Object newObject, JsonReader reader, JsonObjectContract contract, JsonProperty member, String id)
JsonSerializerInternalReader.CreateObject(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
JsonSerializerInternalReader.Deserialize(JsonReader reader, Type objectType, Boolean checkAdditionalContent)
JsonSerializer.DeserializeInternal(JsonReader reader, Type objectType)
JsonSerializer.Deserialize(JsonReader reader, Type objectType)
JsonSerializer.Deserialize[T](JsonReader reader)
JsonNetApiClientSerialization.DeserializeFromStream[T](Stream stream)
JsonNetApiClientSerialization.DeserializeFromStreamAsync[T](Stream stream)
ApiClientBase.DeserializeJsonFromStreamAsync[T](Stream stream)
kealist commented 1 year ago

@DiscoPYF Just wanted to follow up to see if there was anything that could be done in the near term to resolve this.

DiscoPYF commented 1 year ago

Thanks for your contibution @kealist

Progress was a bit slow here last month. I have just created a patch release which I hope will help: https://github.com/ArangoDB-Community/arangodb-net-standard/releases/tag/2.0.1

kealist commented 1 year ago

@DiscoPYF Yep, tested on my end and everything is working fine now. Thank you!