Skip to content

feat(api): api review nits #381

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

Merged
merged 1 commit into from
Dec 18, 2020
Merged

feat(api): api review nits #381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions playwright/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,8 @@
FilePayload = api_types.FilePayload
FloatRect = api_types.FloatRect
Geolocation = api_types.Geolocation
HttpCredentials = api_types.HttpCredentials
PdfMargins = api_types.PdfMargins
ProxySettings = api_types.ProxySettings
RecordHarOptions = api_types.RecordHarOptions
RecordVideoOptions = api_types.RecordVideoOptions
RequestFailure = api_types.RequestFailure
OptionSelector = api_types.OptionSelector
SourceLocation = api_types.SourceLocation
TimeoutError = api_types.TimeoutError

Expand All @@ -55,19 +50,14 @@ def sync_playwright() -> SyncPlaywrightContextManager:
"async_playwright",
"sync_playwright",
"Cookie",
"HttpCredentials",
"DeviceDescriptor",
"Error",
"FilePayload",
"FloatRect",
"Geolocation",
"PdfMargins",
"ProxySettings",
"RecordHarOptions",
"RecordVideoOptions",
"RequestFailure",
"ResourceTiming",
"OptionSelector",
"SourceLocation",
"StorageState",
"TimeoutError",
Expand Down
99 changes: 7 additions & 92 deletions playwright/_api_types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,14 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from pathlib import Path
import sys
from typing import Any, Dict, Optional, Tuple, Union

if sys.version_info >= (3, 8): # pragma: no cover
from typing import TypedDict
else: # pragma: no cover
from typing_extensions import TypedDict


class Error(Exception):
def __init__(self, message: str, stack: str = None) -> None:
Expand Down Expand Up @@ -68,37 +73,13 @@ def __init__(self, x: float, y: float, width: float, height: float):
self.height = height


class DeviceDescriptor(ApiType):
class DeviceDescriptor(TypedDict):
user_agent: Optional[str]
viewport: Optional[Tuple[int, int]]
device_scale_factor: Optional[int]
is_mobile: Optional[bool]
has_touch: Optional[bool]

@classmethod
def _parse(cls, dict: Dict) -> "DeviceDescriptor":
return DeviceDescriptor(
dict["userAgent"],
dict["viewport"],
dict["deviceScaleFactor"],
dict["isMobile"],
dict["hasTouch"],
)

def __init__(
self,
user_agent: str = None,
viewport: Tuple[int, int] = None,
device_scale_factor: int = None,
is_mobile: bool = None,
has_touch: bool = None,
):
self.user_agent = user_agent
self.viewport = viewport
self.device_scale_factor = device_scale_factor
self.is_mobile = is_mobile
self.has_touch = has_touch


class Geolocation(ApiType):
latitude: float
Expand All @@ -111,15 +92,6 @@ def __init__(self, latitude: float, longitude: float, accuracy: float = None):
self.accuracy = accuracy


class HttpCredentials(ApiType):
username: str
password: str

def __init__(self, username: str, password: str):
self.username = username
self.password = password


class PdfMargins(ApiType):
top: Optional[Union[str, int]]
right: Optional[Union[str, int]]
Expand Down Expand Up @@ -158,52 +130,6 @@ def __init__(
self.password = password


class RequestFailure(ApiType):
error_text: str

@classmethod
def _parse(cls, dict: Optional[Dict]) -> Optional["RequestFailure"]:
if not dict:
return None
return RequestFailure(dict["errorText"])

def __init__(self, error_text: str):
self.error_text = error_text


class RecordHarOptions(ApiType):
omit_content: Optional[bool]
path: Union[Path, str]

def __init__(self, path: Union[str, Path], omit_content: bool = None):
self.path = path
self.omit_content = omit_content

def _to_json(self) -> Dict:
return filter_out_none(
{"omitContent": self.omit_content, "path": str(self.path)}
)


class RecordVideoOptions(ApiType):
dir: Union[Path, str]
size: Optional[Tuple[int, int]]

def __init__(self, dir: Union[str, Path], size: Tuple[int, int] = None):
self.dir = dir
self.size = size

def _to_json(self) -> Dict:
return filter_out_none(
{
"dir": str(self.dir),
"size": {"width": self.size[0], "height": self.size[1]}
if self.size
else None,
}
)


class SourceLocation(ApiType):
url: str
line: int
Expand All @@ -215,17 +141,6 @@ def __init__(self, url: str, line: int, column: int):
self.column = column


class OptionSelector(ApiType):
value: Optional[str]
label: Optional[str]
index: Optional[int]

def __init__(self, value: str = None, label: str = None, index: int = None):
self.value = value
self.label = label
self.index = index


def filter_out_none(args: Dict) -> Any:
copy = {}
for key in args:
Expand Down
76 changes: 42 additions & 34 deletions playwright/_browser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,7 @@
from typing import TYPE_CHECKING, Dict, List, Tuple, Union

from playwright._api_structures import StorageState
from playwright._api_types import (
Geolocation,
HttpCredentials,
ProxySettings,
RecordHarOptions,
RecordVideoOptions,
)
from playwright._api_types import Geolocation, ProxySettings
from playwright._browser_context import BrowserContext
from playwright._connection import ChannelOwner, from_channel
from playwright._helper import ColorScheme, is_safe_close_error, locals_to_params
Expand Down Expand Up @@ -83,32 +77,22 @@ async def newContext(
permissions: List[str] = None,
extraHTTPHeaders: Dict[str, str] = None,
offline: bool = None,
httpCredentials: HttpCredentials = None,
httpCredentials: Tuple[str, str] = None,
deviceScaleFactor: int = None,
isMobile: bool = None,
hasTouch: bool = None,
colorScheme: ColorScheme = None,
acceptDownloads: bool = None,
defaultBrowserType: str = None,
proxy: ProxySettings = None,
recordHar: RecordHarOptions = None,
recordVideo: RecordVideoOptions = None,
recordHarPath: Union[Path, str] = None,
recordHarOmitContent: bool = None,
recordVideoDir: Union[Path, str] = None,
recordVideoSize: Tuple[int, int] = None,
storageState: Union[StorageState, str, Path] = None,
) -> BrowserContext:
params = locals_to_params(locals())
# Python is strict in which variables gets passed to methods. We get this
# value from the device descriptors, thats why we have to strip it out.
if defaultBrowserType in params:
del params["defaultBrowserType"]
if storageState:
if not isinstance(storageState, dict):
with open(storageState, "r") as f:
params["storageState"] = json.load(f)
if viewport == 0:
del params["viewport"]
params["noDefaultViewport"] = True
if extraHTTPHeaders:
params["extraHTTPHeaders"] = serialize_headers(extraHTTPHeaders)
normalize_context_params(params)

channel = await self._channel.send("newContext", params)
context = from_channel(channel)
Expand All @@ -130,27 +114,21 @@ async def newPage(
permissions: List[str] = None,
extraHTTPHeaders: Dict[str, str] = None,
offline: bool = None,
httpCredentials: HttpCredentials = None,
httpCredentials: Tuple[str, str] = None,
deviceScaleFactor: int = None,
isMobile: bool = None,
hasTouch: bool = None,
colorScheme: ColorScheme = None,
acceptDownloads: bool = None,
defaultBrowserType: str = None,
proxy: ProxySettings = None,
recordHar: RecordHarOptions = None,
recordVideo: RecordVideoOptions = None,
recordHarPath: Union[Path, str] = None,
recordHarOmitContent: bool = None,
recordVideoDir: Union[Path, str] = None,
recordVideoSize: Tuple[int, int] = None,
storageState: Union[StorageState, str, Path] = None,
) -> Page:
params = locals_to_params(locals())
# Python is strict in which variables gets passed to methods. We get this
# value from the device descriptors, thats why we have to strip it out.
if defaultBrowserType:
del params["defaultBrowserType"]
if storageState:
if not isinstance(storageState, dict):
with open(storageState, "r") as f:
params["storageState"] = json.load(f)
context = await self.newContext(**params)
page = await context.newPage()
page._owned_context = context
Expand All @@ -170,3 +148,33 @@ async def close(self) -> None:
@property
def version(self) -> str:
return self._initializer["version"]


def normalize_context_params(params: Dict) -> None:
if "viewport" in params and params["viewport"] == 0:
del params["viewport"]
params["noDefaultViewport"] = True
if "defaultBrowserType" in params:
del params["defaultBrowserType"]
if "extraHTTPHeaders" in params:
params["extraHTTPHeaders"] = serialize_headers(params["extraHTTPHeaders"])
if "recordHarPath" in params:
params["recordHar"] = {"path": str(params["recordHarPath"])}
if "recordHarOmitContent" in params:
params["recordHar"]["omitContent"] = True
del params["recordHarOmitContent"]
del params["recordHarPath"]
if "recordVideoDir" in params:
params["recordVideo"] = {"dir": str(params["recordVideoDir"])}
if "recordVideoSize" in params:
params["recordVideo"]["size"] = {
"width": params["recordVideoSize"][0],
"height": params["recordVideoSize"][1],
}
del params["recordVideoSize"]
del params["recordVideoDir"]
if "storageState" in params:
storageState = params["storageState"]
if not isinstance(storageState, dict):
with open(storageState, "r") as f:
params["storageState"] = json.load(f)
13 changes: 10 additions & 3 deletions playwright/_browser_context.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from typing import TYPE_CHECKING, Any, Callable, Dict, List, Optional, Union, cast

from playwright._api_structures import Cookie, StorageState
from playwright._api_types import Error, Geolocation
from playwright._api_types import Error
from playwright._connection import ChannelOwner, from_channel
from playwright._event_context_manager import EventContextManagerImpl
from playwright._helper import (
Expand Down Expand Up @@ -141,8 +141,15 @@ async def grantPermissions(
async def clearPermissions(self) -> None:
await self._channel.send("clearPermissions")

async def setGeolocation(self, geolocation: Optional[Geolocation]) -> None:
await self._channel.send("setGeolocation", locals_to_params(locals()))
async def setGeolocation(
self, latitude: float, longitude: float, accuracy: Optional[float]
) -> None:
await self._channel.send(
"setGeolocation", {"geolocation": locals_to_params(locals())}
)

async def resetGeolocation(self) -> None:
await self._channel.send("setGeolocation", {})

async def setExtraHTTPHeaders(self, headers: Dict[str, str]) -> None:
await self._channel.send(
Expand Down
25 changes: 8 additions & 17 deletions playwright/_browser_type.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,11 @@
from pathlib import Path
from typing import Dict, List, Tuple, Union

from playwright._api_types import (
Geolocation,
HttpCredentials,
ProxySettings,
RecordHarOptions,
RecordVideoOptions,
)
from playwright._browser import Browser
from playwright._api_types import Geolocation, ProxySettings
from playwright._browser import Browser, normalize_context_params
from playwright._browser_context import BrowserContext
from playwright._connection import ChannelOwner, from_channel
from playwright._helper import ColorScheme, Env, locals_to_params, not_installed_error
from playwright._network import serialize_headers

if sys.version_info >= (3, 8): # pragma: no cover
from typing import Literal
Expand Down Expand Up @@ -103,23 +96,21 @@ async def launchPersistentContext(
permissions: List[str] = None,
extraHTTPHeaders: Dict[str, str] = None,
offline: bool = None,
httpCredentials: HttpCredentials = None,
httpCredentials: Tuple[str, str] = None,
deviceScaleFactor: int = None,
isMobile: bool = None,
hasTouch: bool = None,
colorScheme: ColorScheme = None,
acceptDownloads: bool = None,
chromiumSandbox: bool = None,
recordHar: RecordHarOptions = None,
recordVideo: RecordVideoOptions = None,
recordHarPath: Union[Path, str] = None,
recordHarOmitContent: bool = None,
recordVideoDir: Union[Path, str] = None,
recordVideoSize: Tuple[int, int] = None,
) -> BrowserContext:
userDataDir = str(Path(userDataDir))
params = locals_to_params(locals())
if viewport == 0:
del params["viewport"]
params["noDefaultViewport"] = True
if extraHTTPHeaders:
params["extraHTTPHeaders"] = serialize_headers(extraHTTPHeaders)
normalize_context_params(params)
normalize_launch_params(params)
try:
context = from_channel(
Expand Down
2 changes: 2 additions & 0 deletions playwright/_connection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,8 @@ def _replace_channels_with_guids(self, payload: Any, param_name: str) -> Any:
return {"x": payload[0], "y": payload[1]}
if param_name == "size" or param_name == "viewport":
return {"width": payload[0], "height": payload[1]}
if param_name == "httpCredentials":
return {"username": payload[0], "password": payload[1]}
if isinstance(payload, Path):
return str(payload)
if isinstance(payload, ApiType):
Expand Down
Loading