-
Notifications
You must be signed in to change notification settings - Fork 16
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
Rewrite API to parse JSON content body in POST and DELETE requests #156
Conversation
Solved Merge conflicts with cuttingedge branch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks more readable than mine.
Left a few things I would improve, but other than that, looks ready to merge (since the old uri-method can still be used)
None | ||
|
||
- Request Body: | ||
None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can have an optional body, to get a specific job
{
jobId: ${Tranga Job ID}
}
See
Lines 172 to 176 in 5f03b0d
if(!_parent.jobBoss.jobs.Any(jjob => jjob.id == jobId)) | |
SendResponse(HttpStatusCode.BadRequest, response); | |
else | |
SendResponse(HttpStatusCode.OK, response, _parent.jobBoss.jobs.First(jjob => jjob.id == jobId)); | |
break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't think of a good reason why we should split Parameters and Body...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's all getting thrown into the same Dictionary, so having a key-value table sounds like a better idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I agree, I'll just make them the same dictionary if that's what you meant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean in the md file you don't have to split them into "Parameters" and "Request Body"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, okay, I was kinda following the Kacita API docs guide, but yeah
- `{jobId}`: Tranga Job ID | ||
|
||
- Request Body: | ||
None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{
"jobId": ${Tranga Job ID}
}
None | ||
|
||
- Request Body: | ||
None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optional, to just update a single manga
{
internalId: ${Tranga Manga ID}
}
} | ||
``` | ||
#### [POST] /Settings/userAgent | ||
Updates the user agent that Tranga uses when scraping the web |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when left empty, resets it to the default
Encoding encoding = request.ContentEncoding; | ||
StreamReader reader = new StreamReader(body, encoding); | ||
string s = reader.ReadToEnd(); | ||
Dictionary<string, string> requestBody = JsonConvert.DeserializeObject<Dictionary<string, string>>(s); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably want a try-catch so we dont crash every time we get a request that has for example integers, booleans, arrays, or nested objects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call
Gonna leave this as draft until I verify all the API calls work properly.
Addresses Rewrite API to use JSON Content Body #144