-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Update OC 1.8.0-preview-17858 to 1.9.x latest #15416
Comments
@MikeAlhayek Yeah, see that's what I meant when I said it will probably not all work magically. 🤡 |
Maybe relate of #14572 |
Reason is the type of the OrchardCore/src/OrchardCore/OrchardCore.Users.Core/Models/User.cs Lines 46 to 50 in e35751d
public class UserLoginInfo
{
/// <summary>
/// Creates a new instance of <see cref="UserLoginInfo"/>
/// </summary>
/// <param name="loginProvider">The provider associated with this login information.</param>
/// <param name="providerKey">The unique identifier for this user provided by the login provider.</param>
/// <param name="displayName">The display name for the login provider.</param>
public UserLoginInfo(string loginProvider, string providerKey, string? displayName)
{
LoginProvider = loginProvider;
ProviderKey = providerKey;
ProviderDisplayName = displayName;
} |
Looks like the
using System;
using System.Text.Json;
using System.Text.Json.Serialization;
using Microsoft.AspNetCore.Identity;
namespace OrchardCore.Users.Json;
public class LoginInfoConverter : JsonConverter<UserLoginInfo>
{
public override UserLoginInfo Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
var loginInfo = new UserLoginInfo(string.Empty, string.Empty, string.Empty);
while (reader.Read())
{
if (reader.TokenType == JsonTokenType.PropertyName)
{
string propertyName = reader.GetString();
reader.Read();
switch (propertyName)
{
case nameof(UserLoginInfo.LoginProvider):
loginInfo.LoginProvider = reader.GetString();
break;
case nameof(UserLoginInfo.ProviderKey):
loginInfo.ProviderKey = reader.GetString();
break;
case nameof(UserLoginInfo.ProviderDisplayName):
loginInfo.ProviderDisplayName = reader.GetString();
break;
default:
break;
}
}
else if (reader.TokenType == JsonTokenType.EndObject)
{
break;
}
}
return loginInfo;
}
public override void Write(Utf8JsonWriter writer,
UserLoginInfo objectToWrite,
JsonSerializerOptions options) =>
JsonSerializer.Serialize(writer, objectToWrite, objectToWrite.GetType(), options);
}
|
OrchardCore.Deployment.DeploymentStep
public class DeploymentPlan
{
public long Id { get; set; }
public string Name { get; set; }
public List<DeploymentStep> DeploymentSteps { get; init; } = [];
}
public abstract class DeploymentStep
{
public string Id { get; set; }
public string Name { get; set; }
} |
OrchardCore.Queries.QuerySystem.NotSupportedException:“Deserialization of types without a parameterless constructor, a singular parameterized constructor, or a parameterized constructor annotated with 'JsonConstructorAttribute' is not supported. Type 'OrchardCore.Queries.Query'. Path: $.Queries.OrderValidation | LineNumber: 0 | BytePositionInLine: 1907.” |
At this point, I was curious, because it's supposed to be impossible to serialize an abstract class or a type without a default constructor. How does Newtownsoft.Json do this? |
This comment was marked as outdated.
This comment was marked as outdated.
@hyzx86 is
From the migration docs https://docs.orchardcore.net/en/latest/docs/releases/1.9.0/
|
After looking int UserLoginInfo it happens that it was not due to missing a default constructor, since it has a single one it's fine too. The issue is in the ctor argument not matching the property name. I created this PR to fix it in next aspnet version. dotnet/aspnetcore#54298 |
So the STJ serialize uses a naming convention to match up properties to constructors? Interesting. |
Hi @MikeAlhayek , Did we miss the asp.net core api serialization configuration? the below code will use diffrent serialize option , just like the top line will ignore null property , the last line will include null value. public async Task<IActionResult> Query(string name)
{
// the below code will use diffrent serialize option , the top line will ignore null property , the last line will include null value
return Content(JConvert.SerializeObject(result));
return new ObjectResult(result);
} Please reopen this issue, my test is not finished yet 😁 |
I mean, under oc1.8x, the default serialization option ignores empty properties |
Chinese content will be encoded instead of the original text1.8x: 1.9x ( need to use |
Before someone changes |
@hyzx86 at what point are you getting this error? The
I am not sure what you mean here. API serialization can have different options that the document serialization. Remember that document serialization is only used by YesSql to serialize objects that are stored in the document table. Once the object is save and returned from YesSql, you are free to do anything you want with it like serialize it and print it out in any format you want. |
The public async Task<IActionResult> Query(string name)
{
// the below code will use diffrent serialize option , the top line will ignore null property , the last line will include null value
return Content(JConvert.SerializeObject(result));
return new ObjectResult(result);
} |
@MikeAlhayek Maybe the question here is that ASP.NET will use STJ to serialize object results from controller/actions, and there must be a way to set the serialization options to use which might be missing right now. c.f. https://www.meziantou.net/configuring-json-options-in-asp-net-core.htm |
reading that comment again and seeing the two return statements in the action, tells me you are right. |
It doesn't matter, I just want to express it in which position and which way will produce different results.
…---Original---
From: "Mike ***@***.***>
Date: Wed, Mar 6, 2024 02:28 AM
To: ***@***.***>;
Cc: "Tony ***@***.******@***.***>;
Subject: Re: [OrchardCMS/OrchardCore] Update OC 1.8.0-preview-17858 to 1.9.xlatest (Issue #15416)
reading that comment again and seeing the two return statements in the action, tells me you are right.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
@sebastienros Yes, that's what I meant . 👍 @MikeAlhayek , I have some unit tests to verify that the client receives the correct results after every upgrade or code change in the OC. Now my client application will break if we don't adjust the code |
The single quote should be recognized as a valid property name qualifierAt least it can be recognized in STJ before var parameters = "{'size':1}`";
var para = JConvert.DeserializeObject<Dictionary<string, object>>(parameters) -- Looks like it won't be supported |
Where do you have property name with single quotes? |
parameters of query and graphql parameters |
In the current version, the data type is not checked when creating content through the Api, so the wrong type can be successfully saved to the database. {
"SalesOrg":{
"Text":1234
}
}
This problem causes the content page to enter an infinite loop
|
Hi @MikeAlhayek , What has changed recently, and this problem occurs when I log in after pulling the latest code
|
If you are injecting IOptions in your prroject, you should change it to inject IOptions instead. |
Yes, I mean I have this problem in the latest version of the source code. |
@MikeAlhayek I updated the log in the comments above Because IStore objects have a singleton life cycle, which means that their configuration is not modified once initialized, serialization options configured in subsequent modules are not executed again OrchardCore/src/OrchardCore/OrchardCore.Data.YesSql/OrchardCoreBuilderExtensions.cs Lines 56 to 70 in 11dc971
But I wonder why there are no problems in the unit tests. |
The breakpoint will never be hit on line 212 OrchardCore/src/OrchardCore/OrchardCore.Data.YesSql/OrchardCoreBuilderExtensions.cs Lines 210 to 213 in 11dc971
|
The problem is that I forgot to update the registration of the user converter. I am out of town with no PC to fix this. If you like, submit a PR that updates the following To this
Request my review on that PR and I'll merge it. If you have any different issue please open a new issue so we can track it separately and avoid commenting on close issues. |
Try accessing the .well-knows url in your browser and see if that gives you an error or not. If there is an issue please open up a new issue. Also I am wondering if the output of the .well-known format changed with the latest changes we made to the global parser. Can you please share the json output you see when you access the .well-known endpoint in your browser? |
@MikeAlhayek No problem, I've already created an Issue But let's take a look at this question. Should we update our JSON serialization to be compatible with the wrong data format? Also, when I used the database statement to fix the data type, I still found some encoding problems, Chinese and special symbols were encoded UPDATE Document
SET Content = JSON_MODIFY(Content, '$.SEAQuotationLogics.SalesOrg.Text', REPLACE(CAST(JSON_VALUE(d.content, '$.SEAQuotationLogics.SalesOrg.Text') AS NVARCHAR(MAX)),'.0',''))
FROM Dindex_SEAQuotationLogics l
INNER JOIN Document d ON l.DocumentId = d.Id; |
@sebastienros thoughts on the last comment? |
It looks like there's no way to read a number as a string in STJ. Maybe we should adjust our code to accommodate this? How about adding a type check when reading I checked the code and it feels like it's not easy or expensive, but if we can't do anything, we should document how to handle this change |
Why? Not sure if I fully understand that issue you are having. TextField by definition is for text/string and should always render string even if the value is a numeric format. If you want to capture numeric, then you should use the NumericFiels. |
As I mentioned before, they are created using webApi, skipping |
You can add field handler to do the same validation as the validation done by the driver. But this should not be a STJ related |
@hyzx86 请问一下,2.0.2版本中,中文被转义的问题,现在有办法解决了吗? |
如果是API结果的问题,你可以覆盖MVC 的序列化选项,对应位置修改对应的序列化选项就好了 |
Thanks! return Content(JConvert.SerializeObject(sresult, JOptions.UnsafeRelaxedJsonEscaping)); |
My data works fine under version
1.8.0-preview-17858
, but after upgrading to the latest version 1.9x, I have the following problem when I log in. Theadmin
account looks fine, but this exception is also thrown when I open the user listUser Document
Details:
The text was updated successfully, but these errors were encountered: