This repository was archived by the owner on Feb 22, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[webview_flutter] Add platform interface method loadRequest
.
#4450
Merged
fluttergithubbot
merged 6 commits into
flutter:master
from
Baseflow:webview_flutter/request_url_platform
Nov 2, 2021
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
53eed81
Add platform interface method `loadRequest`.
BeMacized d7ffb45
Add test
BeMacized 6cf6411
Add missing license
BeMacized 3bd7d4b
Add test
BeMacized b927414
Add missing test
BeMacized 9e0b5ff
Merge branch 'master' into webview_flutter/request_url_platform
BeMacized File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
4 changes: 4 additions & 0 deletions
4
packages/webview_flutter/webview_flutter_platform_interface/CHANGELOG.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
58 changes: 58 additions & 0 deletions
58
...ges/webview_flutter/webview_flutter_platform_interface/lib/src/types/webview_request.dart
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
// Copyright 2013 The Flutter Authors. All rights reserved. | ||
// Use of this source code is governed by a BSD-style license that can be | ||
// found in the LICENSE file. | ||
|
||
import 'dart:typed_data'; | ||
|
||
/// Defines the supported HTTP methods for loading a page in [WebView]. | ||
enum WebViewRequestMethod { | ||
/// HTTP GET method. | ||
get, | ||
|
||
/// HTTP POST method. | ||
post, | ||
} | ||
|
||
/// Extension methods on the [WebViewRequestMethod] enum. | ||
extension WebViewRequestMethodExtensions on WebViewRequestMethod { | ||
/// Converts [WebViewRequestMethod] to [String] format. | ||
String serialize() { | ||
switch (this) { | ||
case WebViewRequestMethod.get: | ||
return 'get'; | ||
case WebViewRequestMethod.post: | ||
return 'post'; | ||
} | ||
} | ||
} | ||
|
||
/// Defines the parameters that can be used to load a page in the [WebView]. | ||
class WebViewRequest { | ||
/// Creates the [WebViewRequest]. | ||
WebViewRequest({ | ||
required this.uri, | ||
required this.method, | ||
this.headers = const {}, | ||
this.body, | ||
}); | ||
|
||
/// URI for the request. | ||
final Uri uri; | ||
|
||
/// HTTP method used to make the request. | ||
final WebViewRequestMethod method; | ||
|
||
/// Headers for the request. | ||
final Map<String, String> headers; | ||
|
||
/// HTTP body for the request. | ||
final Uint8List? body; | ||
|
||
/// Serializes the [WebViewRequest] to JSON. | ||
Map<String, dynamic> toJson() => { | ||
'uri': this.uri.toString(), | ||
'method': this.method.serialize(), | ||
'headers': this.headers, | ||
'body': this.body, | ||
}; | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
39 changes: 39 additions & 0 deletions
39
...bview_flutter/webview_flutter_platform_interface/test/src/types/webview_request_test.dart
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
// Copyright 2013 The Flutter Authors. All rights reserved. | ||
// Use of this source code is governed by a BSD-style license that can be | ||
// found in the LICENSE file. | ||
|
||
import 'dart:typed_data'; | ||
import 'package:flutter_test/flutter_test.dart'; | ||
import 'package:webview_flutter_platform_interface/src/types/types.dart'; | ||
|
||
void main() { | ||
test('WebViewRequestMethod should serialize correctly', () { | ||
expect(WebViewRequestMethod.get.serialize(), 'get'); | ||
expect(WebViewRequestMethod.post.serialize(), 'post'); | ||
}); | ||
|
||
test('WebViewRequest should serialize correctly', () { | ||
WebViewRequest request; | ||
Map<String, dynamic> serializedRequest; | ||
// Test serialization without headers or a body | ||
request = WebViewRequest( | ||
uri: Uri.parse('https://flutter.dev'), | ||
method: WebViewRequestMethod.get, | ||
); | ||
serializedRequest = request.toJson(); | ||
expect(serializedRequest['uri'], 'https://flutter.dev'); | ||
expect(serializedRequest['method'], 'get'); | ||
expect(serializedRequest['headers'], {}); | ||
expect(serializedRequest['body'], null); | ||
// Test serialization of headers and body | ||
request = WebViewRequest( | ||
uri: Uri.parse('https://flutter.dev'), | ||
method: WebViewRequestMethod.get, | ||
headers: {'foo': 'bar'}, | ||
body: Uint8List.fromList('Example Body'.codeUnits), | ||
); | ||
serializedRequest = request.toJson(); | ||
expect(serializedRequest['headers'], {'foo': 'bar'}); | ||
expect(serializedRequest['body'], 'Example Body'.codeUnits); | ||
}); | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Please add a second test that sets the optional parameters and ensures they come through as expected as well.
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.
@stuartmorgan Would that not just be testing WebViewRequest's
toJson
method? We can of course add a second test, but as thattoJson
is already being tested here, including the optional parameters, I feel like we would be testing for the same thing twice.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.
How do you know that
loadRequest
callsWebViewRequest
'stoJson
method? Or to be more precise: how do you know that it will continue to do so?I guarantee you that I could write a PR that breaks the test I am asking you to write without changing
WebViewRequest.toJson
.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.
That is indeed a fair point, I now see it wouldn't. I've added the test for the optional parameters.
Just for my own curiosity: Would it not be possible to avoid testing
toJson
by instead mocking the WebViewRequest and checking if the return value oftoJson
gets passed toinvokeMethod
?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.
Tests should assert the behavior you want to guarantee. Which is a better description of the thing you want to guarantee is true:
toJson
oninvokeMethod
"?(As an example of why only one of these is a good description: if someone later decides that the JSON keys aren't very descriptive, and sends a PR that updates
toJson
and its test to use different keys, what do you want to happen?)