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.

Sign up for GitHub

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

Jump to bottom

Added custom header support (such as Auth, User-agent, Connection) #381

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

Added custom header support (such as Auth, User-agent, Connection) #381

wants to merge 5 commits into from

Conversation

zorigoo0128
Copy link

No description provided.

trdwll
trdwll suggested changes May 2, 2022
View reviewed changes
Source/VaRest/Private/VaRestSubsystem.cpp Outdated
@@ -30,7 +30,7 @@ void UVaRestSubsystem::Deinitialize()
Super::Deinitialize();
}

void UVaRestSubsystem::CallURL(const FString& URL, EVaRestRequestVerb Verb, EVaRestRequestContentType ContentType, UVaRestJsonObject* VaRestJson, const FVaRestCallDelegate& Callback)
void UVaRestSubsystem::CallURL(const FString& URL, EVaRestRequestVerb Verb, TMap<FString, FString> headers, EVaRestRequestContentType ContentType, UVaRestJsonObject* VaRestJson, const FVaRestCallDelegate& Callback)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be a const reference since it's not being modified.

Source/VaRest/Private/VaRestSubsystem.cpp Outdated
Comment on lines 47 to 49
for (const TPair<FString, FString>& pair : headers)
Request->SetHeader(pair.Key, pair.Value);

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add {} to match the rest of the code?

Source/VaRest/Public/VaRestSubsystem.h Outdated
@@ -15,10 +15,11 @@ DECLARE_DYNAMIC_DELEGATE_OneParam(FVaRestCallDelegate, UVaRestRequestJSON*, Requ
USTRUCT()
struct FVaRestCallResponse
{
GENERATED_BODY()
GENERATED_USTRUCT_BODY()
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be GENERATED_BODY as GENERATED_USTRUCT_BODY is old and deprecated.

Source/VaRest/Public/VaRestSubsystem.h Outdated
@@ -51,7 +52,7 @@ class VAREST_API UVaRestSubsystem : public UEngineSubsystem
public:
/** Easy way to process http requests */
UFUNCTION(BlueprintCallable, Category = "VaRest|Utility")
void CallURL(const FString& URL, EVaRestRequestVerb Verb, EVaRestRequestContentType ContentType, UVaRestJsonObject* VaRestJson, const FVaRestCallDelegate& Callback);
void CallURL(const FString& URL, EVaRestRequestVerb Verb, TMap<FString, FString> headers, EVaRestRequestContentType ContentType, UVaRestJsonObject* VaRestJson, const FVaRestCallDelegate& Callback);
Copy link
Contributor

Choose a reason for hiding this comment

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

const ref

@zorigoo0128
Copy link
Author

Thanks you for your comment.

@zorigoo0128
Copy link
Author

just updated

trdwll
trdwll reviewed Jun 19, 2022
View reviewed changes
Source/VaRest/Private/VaRestSubsystem.cpp Outdated
@@ -30,7 +30,7 @@ void UVaRestSubsystem::Deinitialize()
Super::Deinitialize();
}

void UVaRestSubsystem::CallURL(const FString& URL, EVaRestRequestVerb Verb, EVaRestRequestContentType ContentType, UVaRestJsonObject* VaRestJson, const FVaRestCallDelegate& Callback)
void UVaRestSubsystem::CallURL(const FString& URL, EVaRestRequestVerb Verb, const TMap<FString, FString> headers, EVaRestRequestContentType ContentType, UVaRestJsonObject* VaRestJson, const FVaRestCallDelegate& Callback)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you make the headers tmap a const ref?

trdwll
trdwll reviewed Jun 19, 2022
View reviewed changes
Source/VaRest/Public/VaRestSubsystem.h Outdated
@@ -51,7 +52,7 @@ class VAREST_API UVaRestSubsystem : public UEngineSubsystem
public:
/** Easy way to process http requests */
UFUNCTION(BlueprintCallable, Category = "VaRest|Utility")
void CallURL(const FString& URL, EVaRestRequestVerb Verb, EVaRestRequestContentType ContentType, UVaRestJsonObject* VaRestJson, const FVaRestCallDelegate& Callback);
void CallURL(const FString& URL, EVaRestRequestVerb Verb, const TMap<FString, FString> headers, EVaRestRequestContentType ContentType, UVaRestJsonObject* VaRestJson, const FVaRestCallDelegate& Callback);
Copy link
Contributor

@trdwll trdwll Jun 19, 2022 •
edited

Choose a reason for hiding this comment

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

tmap as const ref

@zorigoo0128
Copy link
Author

zorigoo0128 commented Jun 20, 2022 •
edited

Sorry for the const ref 😅 (updated)

@zorigoo0128
Copy link
Author

what is TeamCity ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trdwll trdwll trdwll approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@zorigoo0128 @trdwll