Skip to content
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 the jint options to support System.text.json #15449

Merged
merged 14 commits into from
Mar 14, 2024

Conversation

hyzx86
Copy link
Contributor

@hyzx86 hyzx86 commented Mar 4, 2024

try to reproduce System.InvalidOperationException:“The node must be of type 'JsonObject'.”

image

@MikeAlhayek
Copy link
Member

Your test case here seems to pass. If the issue related to your code?

@hyzx86
Copy link
Contributor Author

hyzx86 commented Mar 5, 2024

The problem recurred. My previous tests were broken, I updated them, and now the problem recurred.

            var findUser = new GlobalMethod
            {
                Name = "getUserByName",
                Method = sp => (string userName) =>
                {
                    var userManager = sp.GetRequiredService<UserManager<IUser>>();
                    var userInfo = userManager.FindByNameAsync(userName).GetAwaiter().GetResult();
                    var jobjUser = JObject.FromObject(userInfo, JOptions.CamelCase);
                    jobjUser.Remove("securityStamp");
                    jobjUser.Remove("passwordHash");
                    jobjUser.Remove("resetToken");
                    jobjUser.Remove("userTokens");
                    return jobjUser;
                }
            };

            var scriptingEngine = scope.ServiceProvider.GetRequiredService<IScriptingEngine>();
            var scriptingScope = scriptingEngine.CreateScope([findUser], scope.ServiceProvider, null, null);
            var result = (bool)scriptingEngine.Evaluate(scriptingScope, "var user = getUserByName('admin'); return user.userName == 'admin'");
            Assert.True(result);

@hyzx86 hyzx86 changed the title Add ScriptTest function Update the Jint option to support System.text.json Mar 5, 2024
@hyzx86 hyzx86 changed the title Update the Jint option to support System.text.json Update the JInt options to support System.text.json Mar 5, 2024
@hyzx86 hyzx86 changed the title Update the JInt options to support System.text.json Update the jint options to support System.text.json Mar 6, 2024
@hyzx86
Copy link
Contributor Author

hyzx86 commented Mar 6, 2024

Hi @MikeAlhayek , this PR is ready to merge

@hyzx86
Copy link
Contributor Author

hyzx86 commented Mar 6, 2024

@MikeAlhayek Done.

@hyzx86 hyzx86 requested a review from MikeAlhayek March 6, 2024 16:40
Name = "tryAccessJsonObject",
Method = sp => () =>
{
return new JsonObject
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you try adding other STJ types in the test, like arrays, bool, numbers, to be sure it works fine too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

@hyzx86 hyzx86 Mar 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that false value in JsonObject does not work as expected

sebastienros/jint#1767 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hyzx86 hyzx86 marked this pull request as draft March 13, 2024 03:59
@hyzx86 hyzx86 marked this pull request as ready for review March 13, 2024 09:59
@MikeAlhayek MikeAlhayek merged commit b83e2dc into OrchardCMS:main Mar 14, 2024
4 checks passed
urbanit pushed a commit to urbanit/OrchardCore that referenced this pull request Mar 18, 2024
@hyzx86 hyzx86 deleted the add_usertestfunction branch June 13, 2024 06:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants