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

Added Optional Custom Serializer in Soap Core #973

Merged
merged 11 commits into from
Jun 25, 2024

Conversation

ankitkmrpatel
Copy link
Contributor

Added Custom Soap Core Serializer using DI
If Custom Soap Core Serializer NOT found, Then Fall back for default Soap Core Serializer

@ankitkmrpatel ankitkmrpatel changed the title Added Optional Custom Serializer in Soap Core #968 Added Optional Custom Serializer in Soap Core Oct 19, 2023
@andersjonsson
Copy link
Collaborator

Please fix the build errors in the test project and I will take a look.
I would also like you to add a test case to make sure that it works and won't break by future changes

@andersjonsson
Copy link
Collaborator

And thank you for the PR 😄

@ankitkmrpatel
Copy link
Contributor Author

Hi,

Fixed build error(s) for Middleware in test proj and added new test case for Serializer.

Let me know if there is anything to add.

@andersjonsson
Copy link
Collaborator

There are plenty of failing tests. Please take a look and fix that

----Added delegate To Get the Custom Service if Added Into SoapOptions
----Added Exception if Custom Impl NOT Found
@ankitkmrpatel
Copy link
Contributor Author

Fixed! and Updated Docs for Latest Code.

@ankitkmrpatel
Copy link
Contributor Author

@andersjonsson any comments?

@andersjonsson
Copy link
Collaborator

I think it looks fine. I would like you to change the name though. SoapCoreSerializer is a bit too close to SoapSerializer. Since it serializes input parameters I think that should be reflected in the name in some way.

@ankitkmrpatel
Copy link
Contributor Author

Hi @andersjonsson

Changed from ISoapCoreSerializer to IXmlSerializationHandler, which emphasizes serialization without explicitly mentioning "input parameter" while still conveying the purpose of the interface.

@andersjonsson
Copy link
Collaborator

I think I'm ready to merge this. There are conflicts in SoapCoreOptions.cs that has to be resolved first though

@ankitkmrpatel
Copy link
Contributor Author

@andersjonsson any news on this?

@andersjonsson
Copy link
Collaborator

The conflicts are still there. Please resolve them, then I can merge this branch

@ankitkmrpatel
Copy link
Contributor Author

@andersjonsson Resolved the conflicts and updated the code. Could you look and let me know if you need anything else?

@andersjonsson andersjonsson merged commit 5b4dfe2 into DigDes:develop Jun 25, 2024
3 checks passed
@andersjonsson
Copy link
Collaborator

Thank you!

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.

2 participants