-
-
Notifications
You must be signed in to change notification settings - Fork 749
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
Relay Guid Identifier Unable to convert type from Object
to Guid
#7110
Comments
Thanks for the repro ... we will have a look at this. |
Is there any status on this? |
this is still an issue with 130 |
I ran into the same issue. I already start to dig into the problem and created a unittest which reproduces the problem. It seems that the ids are deserialized two times. I hope to fix this problem soon and will do a pr then. |
I created a branch which reproduces the problem https://github.com/ChilliCream/graphql-platform/tree/mha/7110-id-filter-bug. The problem seems to be that Member type is null in RelayIdFilterFieldExtensions. That's why a wrong Id serializer is created. So far I did not find the right spot where the Member should be set because my knowledge of the code is not good enough. I will try to find the solution but maybe somehow knows it right away.
|
@michaelstaib I am sorry. I don't find the correct spot. Maybe the unittest and the text above helps you to find the bug. Give me a ping if there is anything I can do. |
After further investigation, I was able to resolve the issue, though I don't have enough context to be sure it won't cause issues elsewhere. It seems the problem lies in the if (serializer.ValueSerializer.IsSupported(runtimeType))
{
valueSerializer = serializer.ValueSerializer;
}
else if (valueSerializer is null)
{
valueSerializer = TryResolveSerializer(runtimeType);
}
if (valueSerializer is null)
{
throw SerializerMissing(typeName, rentedBuffer);
}
var value = ParseValue(valueSerializer, serializer.TypeName, span.Slice(delimiterIndex + delimiterOffset));
Clear(rentedBuffer);
return value; In this case, While this works for types like integers, it doesn't for GUIDs when using the new compressed format, which is enabled by default. When the compressed string is later processed to convert it to a GUID, it fails giving us the To fix this, I think we need to modify the @michaelstaib If this approach seems reasonable and you're not concerned about it causing any issues, I'm happy to submit a PR. |
This would not fix the issue just just put a workaround in because another component passes on the wrong type information. The issue is with filtering, and we have to fix it there. |
In filtering probably the runtime type is not set, as far as I can tell from this issue. |
@A360JMaxxgamer was on the right track. |
@michaelstaib I see. In that case, it looks like the public IFilterOperationFieldDescriptor Operation(int operationId)
{
var fieldDescriptor =
Operations.FirstOrDefault(t => t.Definition.Id == operationId);
if (fieldDescriptor is null)
{
fieldDescriptor = FilterOperationFieldDescriptor.New(
Context,
operationId,
Definition.Scope);
Operations.Add(fieldDescriptor);
}
return fieldDescriptor;
} |
I created a pr. It fixes the issue but it feels a bit hacky. I did not find a better solution. But I now understand the problem. before we tried to set the member data which was used to deserialize the value. Actually we could never use that or set that. The member type on the filterfieldoperation can be different for various inputs. The field type for e.g. "nin" on the schema is always IdFilterOperationNin therefore we can not determine the runtime type for a specific id when we complete that type. I tried to move the logic to the input parser. The input parser has a target type. So when the parser sees a formatter for ids it tries to pass the targetType to it. |
@A360JMaxxgamer I will have a look through the PR and then we see how we can make it work. |
Checking in on this issue. I just came across this when upgrading to 14.1.0. My scenario is slightly different but also related to deserializing Guids from a node id. |
@jbenettius I had the same error message. I was able to fix it by registering the serializer with usage of url safe base64 encoding
|
@A360JMaxxgamer thank you for the recommendation, but unfortunately that did not work for me. |
The .AddDefaultNodeIdSerializer(useUrlSafeBase64: true) did not work for me either on version 14.0. What did work was turning off the new id format. |
@adraut thank you for the suggestion, but that also did not work for me. I was able to successful deserialize the Id by implementing my own INodeIdValueSerializer, for Guid, changing the parsing to the |
@michaelstaib |
workaround for those who want to specify custom // add custom node id value serializer
internal sealed class GuidCustomNodeIdValueSerializer : INodeIdValueSerializer
{
public bool IsSupported(Type type) => type == typeof(Guid) || type == typeof(Guid?);
public NodeIdFormatterResult Format(Span<byte> buffer, object value, out int written)
{
if (value is Guid g)
{
return Utf8Formatter.TryFormat(g, buffer, out written, format: 'D') // desired format
? NodeIdFormatterResult.Success
: NodeIdFormatterResult.BufferTooSmall;
}
written = 0;
return NodeIdFormatterResult.InvalidValue;
}
public bool TryParse(ReadOnlySpan<byte> buffer, [NotNullWhen(true)] out object? value)
{
if (Utf8Parser.TryParse(buffer, out Guid parsedValue, out _, 'D')) // specify format here (or pass as constructor param)
{
value = parsedValue;
return true;
}
value = null;
return false;
}
}
// where DI for hotchocolate is configured
services
.AddNodeIdValueSerializer<GuidCustomNodeIdValueSerializer>()
.AddGlobalObjectIdentification();
//...other registrations
// remove default `GuidNodeIdValueSerializer` which is internal
services
.Where(t =>
t.ServiceType == typeof(INodeIdValueSerializer)
&& t.ImplementationInstance != null
&& t.ImplementationInstance.GetType().Name.Contains("GuidNodeIdValueSerializer"))
.ToList()
.ForEach(s => services.Remove(s)); |
Product
Hot Chocolate
Version
14.0.0-p.100
Link to minimal reproduction
https://github.com/PHILLIPS71/HC-7110
Steps to reproduce
I've previously used Guid identifiers in relay without any issues, though after upgrading to version 14 it doesn't seem to work anymore.
What is expected?
The relay identifiers can be converted into a GUID
What is actually happening?
Relevant log output
No response
Additional context
If you decode
TGlicmFyeTq+UuauxXXqSp5/h9rbTMR0
it looks invalid (Library:RuJ�Lt
)The text was updated successfully, but these errors were encountered: