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

Feature- multipart data name to be specified at the moment of execution. #1002

Merged
merged 6 commits into from Nov 24, 2020

Conversation

farcasclaudiu
Copy link
Contributor

Feature - multipart data name to be specified at the moment of execution.

What kind of change does this PR introduce?
Feature

What is the current behavior?
On current version parameter name or [AliasAs] attribute is used to specify multipart data name. This behavior is not allowing to have multipart data name that is known only at the execution time.

What is the new behavior?
The multipart data name will be taken from streamPart.FileName and can be specified at runtime (dynamically) when the multipart data name is not know ahead of time. If streamPart.FileName is empty ("") it would behave like in previous version (parameter name or [AliasAs]).

What might this PR break?
Because streamPart.FileName will take priority it could break any usage where both streamPart.FileName and [AliasAs] (or parameter name) are used.

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Other information:
I needed this change into a project where multipart was containing a presentation part and multiple binary parts.
Names of the binary parts needed to be dynamically constructed at runtime and the presentation part has dynamic references to the binary parts.

@dnfadmin
Copy link

dnfadmin commented Nov 23, 2020

CLA assistant check
All CLA requirements met.

@clairernovotny
Copy link
Member

Overall, it makes sense to be able to specify the data name dynamically. I'm not sure we want to conflate the filename with the data name. I think it would be better to add a new property to StreamPart, along with a new constructor overload, where the user can specify it if desired. The default would be empty, so the existing tests and behavior wouldn't change.

Copy link
Member

@clairernovotny clairernovotny left a comment

Choose a reason for hiding this comment

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

See my previous comment

@clairernovotny
Copy link
Member

clairernovotny commented Nov 23, 2020

Can we combine this with the PR (#973) from @javirszhang where the change is on MultipartItem so that it works for more than Streams?

@farcasclaudiu
Copy link
Contributor Author

My intention was to have a minimal impact possible on the code, but I can have a look on PR (#973) and eventually adapt it (to cover the need for dynamic datapart name).
Either way, there needs to be created a couple of unit tests to cover this new property usage scenario and the right priority between these various ways of datapart naming. I guess that this new property will have top priority, second will be [AliasAs] and lastly the parameter name.
Plus updating the main documentation (README.md file) to acknowledge this new behavior and priorities.

@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants