From 99890b57bc0da314b0dfa3be2cb6ceba04446a2b Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Fri, 19 Feb 2021 14:34:01 -0800 Subject: [PATCH] fix: generate helper builder methods for nested types (#294) --- .../com/microsoft/playwright/Browser.java | 28 +++++- .../com/microsoft/playwright/BrowserType.java | 17 +++- .../microsoft/playwright/ElementHandle.java | 21 ++-- .../java/com/microsoft/playwright/Frame.java | 21 ++-- .../java/com/microsoft/playwright/Page.java | 24 +++-- .../playwright/TestBrowserContextProxy.java | 5 +- .../microsoft/playwright/TestScreencast.java | 6 +- .../playwright/tools/ApiGenerator.java | 97 ++++++++----------- 8 files changed, 123 insertions(+), 96 deletions(-) diff --git a/playwright/src/main/java/com/microsoft/playwright/Browser.java b/playwright/src/main/java/com/microsoft/playwright/Browser.java index 45772113..e2934beb 100644 --- a/playwright/src/main/java/com/microsoft/playwright/Browser.java +++ b/playwright/src/main/java/com/microsoft/playwright/Browser.java @@ -158,6 +158,9 @@ public interface Browser extends AutoCloseable { this.extraHTTPHeaders = extraHTTPHeaders; return this; } + public NewContextOptions withGeolocation(double latitude, double longitude) { + return withGeolocation(new Geolocation(latitude, longitude)); + } public NewContextOptions withGeolocation(Geolocation geolocation) { this.geolocation = geolocation; return this; @@ -167,7 +170,10 @@ public interface Browser extends AutoCloseable { return this; } public NewContextOptions withHttpCredentials(String username, String password) { - this.httpCredentials = new HttpCredentials(username, password); + return withHttpCredentials(new HttpCredentials(username, password)); + } + public NewContextOptions withHttpCredentials(HttpCredentials httpCredentials) { + this.httpCredentials = httpCredentials; return this; } public NewContextOptions withIgnoreHTTPSErrors(boolean ignoreHTTPSErrors) { @@ -194,6 +200,9 @@ public interface Browser extends AutoCloseable { this.permissions = permissions; return this; } + public NewContextOptions withProxy(String server) { + return withProxy(new Proxy(server)); + } public NewContextOptions withProxy(Proxy proxy) { this.proxy = proxy; return this; @@ -210,6 +219,9 @@ public interface Browser extends AutoCloseable { this.recordVideoDir = recordVideoDir; return this; } + public NewContextOptions withRecordVideoSize(int width, int height) { + return withRecordVideoSize(new RecordVideoSize(width, height)); + } public NewContextOptions withRecordVideoSize(RecordVideoSize recordVideoSize) { this.recordVideoSize = recordVideoSize; return this; @@ -365,6 +377,9 @@ public interface Browser extends AutoCloseable { this.extraHTTPHeaders = extraHTTPHeaders; return this; } + public NewPageOptions withGeolocation(double latitude, double longitude) { + return withGeolocation(new Geolocation(latitude, longitude)); + } public NewPageOptions withGeolocation(Geolocation geolocation) { this.geolocation = geolocation; return this; @@ -374,7 +389,10 @@ public interface Browser extends AutoCloseable { return this; } public NewPageOptions withHttpCredentials(String username, String password) { - this.httpCredentials = new HttpCredentials(username, password); + return withHttpCredentials(new HttpCredentials(username, password)); + } + public NewPageOptions withHttpCredentials(HttpCredentials httpCredentials) { + this.httpCredentials = httpCredentials; return this; } public NewPageOptions withIgnoreHTTPSErrors(boolean ignoreHTTPSErrors) { @@ -401,6 +419,9 @@ public interface Browser extends AutoCloseable { this.permissions = permissions; return this; } + public NewPageOptions withProxy(String server) { + return withProxy(new Proxy(server)); + } public NewPageOptions withProxy(Proxy proxy) { this.proxy = proxy; return this; @@ -417,6 +438,9 @@ public interface Browser extends AutoCloseable { this.recordVideoDir = recordVideoDir; return this; } + public NewPageOptions withRecordVideoSize(int width, int height) { + return withRecordVideoSize(new RecordVideoSize(width, height)); + } public NewPageOptions withRecordVideoSize(RecordVideoSize recordVideoSize) { this.recordVideoSize = recordVideoSize; return this; diff --git a/playwright/src/main/java/com/microsoft/playwright/BrowserType.java b/playwright/src/main/java/com/microsoft/playwright/BrowserType.java index 61900387..272fbb69 100644 --- a/playwright/src/main/java/com/microsoft/playwright/BrowserType.java +++ b/playwright/src/main/java/com/microsoft/playwright/BrowserType.java @@ -155,6 +155,9 @@ public interface BrowserType { this.ignoreDefaultArgs = ignoreDefaultArgs; return this; } + public LaunchOptions withProxy(String server) { + return withProxy(new Proxy(server)); + } public LaunchOptions withProxy(Proxy proxy) { this.proxy = proxy; return this; @@ -375,6 +378,9 @@ public interface BrowserType { this.extraHTTPHeaders = extraHTTPHeaders; return this; } + public LaunchPersistentContextOptions withGeolocation(double latitude, double longitude) { + return withGeolocation(new Geolocation(latitude, longitude)); + } public LaunchPersistentContextOptions withGeolocation(Geolocation geolocation) { this.geolocation = geolocation; return this; @@ -400,7 +406,10 @@ public interface BrowserType { return this; } public LaunchPersistentContextOptions withHttpCredentials(String username, String password) { - this.httpCredentials = new HttpCredentials(username, password); + return withHttpCredentials(new HttpCredentials(username, password)); + } + public LaunchPersistentContextOptions withHttpCredentials(HttpCredentials httpCredentials) { + this.httpCredentials = httpCredentials; return this; } public LaunchPersistentContextOptions withIgnoreAllDefaultArgs(boolean ignoreAllDefaultArgs) { @@ -435,6 +444,9 @@ public interface BrowserType { this.permissions = permissions; return this; } + public LaunchPersistentContextOptions withProxy(String server) { + return withProxy(new Proxy(server)); + } public LaunchPersistentContextOptions withProxy(Proxy proxy) { this.proxy = proxy; return this; @@ -451,6 +463,9 @@ public interface BrowserType { this.recordVideoDir = recordVideoDir; return this; } + public LaunchPersistentContextOptions withRecordVideoSize(int width, int height) { + return withRecordVideoSize(new RecordVideoSize(width, height)); + } public LaunchPersistentContextOptions withRecordVideoSize(RecordVideoSize recordVideoSize) { this.recordVideoSize = recordVideoSize; return this; diff --git a/playwright/src/main/java/com/microsoft/playwright/ElementHandle.java b/playwright/src/main/java/com/microsoft/playwright/ElementHandle.java index 6cff8464..28f1648e 100644 --- a/playwright/src/main/java/com/microsoft/playwright/ElementHandle.java +++ b/playwright/src/main/java/com/microsoft/playwright/ElementHandle.java @@ -145,13 +145,13 @@ public interface ElementHandle extends JSHandle { this.noWaitAfter = noWaitAfter; return this; } + public ClickOptions withPosition(double x, double y) { + return withPosition(new Position(x, y)); + } public ClickOptions withPosition(Position position) { this.position = position; return this; } - public ClickOptions withPosition(double x, double y) { - return withPosition(new Position(x, y)); - } public ClickOptions withTimeout(double timeout) { this.timeout = timeout; return this; @@ -212,13 +212,13 @@ public interface ElementHandle extends JSHandle { this.noWaitAfter = noWaitAfter; return this; } + public DblclickOptions withPosition(double x, double y) { + return withPosition(new Position(x, y)); + } public DblclickOptions withPosition(Position position) { this.position = position; return this; } - public DblclickOptions withPosition(double x, double y) { - return withPosition(new Position(x, y)); - } public DblclickOptions withTimeout(double timeout) { this.timeout = timeout; return this; @@ -275,13 +275,13 @@ public interface ElementHandle extends JSHandle { this.modifiers = modifiers; return this; } + public HoverOptions withPosition(double x, double y) { + return withPosition(new Position(x, y)); + } public HoverOptions withPosition(Position position) { this.position = position; return this; } - public HoverOptions withPosition(double x, double y) { - return withPosition(new Position(x, y)); - } public HoverOptions withTimeout(double timeout) { this.timeout = timeout; return this; @@ -471,6 +471,9 @@ public interface ElementHandle extends JSHandle { this.noWaitAfter = noWaitAfter; return this; } + public TapOptions withPosition(double x, double y) { + return withPosition(new Position(x, y)); + } public TapOptions withPosition(Position position) { this.position = position; return this; diff --git a/playwright/src/main/java/com/microsoft/playwright/Frame.java b/playwright/src/main/java/com/microsoft/playwright/Frame.java index e54e4bbd..7719b425 100644 --- a/playwright/src/main/java/com/microsoft/playwright/Frame.java +++ b/playwright/src/main/java/com/microsoft/playwright/Frame.java @@ -192,13 +192,13 @@ public interface Frame { this.noWaitAfter = noWaitAfter; return this; } + public ClickOptions withPosition(double x, double y) { + return withPosition(new Position(x, y)); + } public ClickOptions withPosition(Position position) { this.position = position; return this; } - public ClickOptions withPosition(double x, double y) { - return withPosition(new Position(x, y)); - } public ClickOptions withTimeout(double timeout) { this.timeout = timeout; return this; @@ -259,13 +259,13 @@ public interface Frame { this.noWaitAfter = noWaitAfter; return this; } + public DblclickOptions withPosition(double x, double y) { + return withPosition(new Position(x, y)); + } public DblclickOptions withPosition(Position position) { this.position = position; return this; } - public DblclickOptions withPosition(double x, double y) { - return withPosition(new Position(x, y)); - } public DblclickOptions withTimeout(double timeout) { this.timeout = timeout; return this; @@ -392,13 +392,13 @@ public interface Frame { this.modifiers = modifiers; return this; } + public HoverOptions withPosition(double x, double y) { + return withPosition(new Position(x, y)); + } public HoverOptions withPosition(Position position) { this.position = position; return this; } - public HoverOptions withPosition(double x, double y) { - return withPosition(new Position(x, y)); - } public HoverOptions withTimeout(double timeout) { this.timeout = timeout; return this; @@ -638,6 +638,9 @@ public interface Frame { this.noWaitAfter = noWaitAfter; return this; } + public TapOptions withPosition(double x, double y) { + return withPosition(new Position(x, y)); + } public TapOptions withPosition(Position position) { this.position = position; return this; diff --git a/playwright/src/main/java/com/microsoft/playwright/Page.java b/playwright/src/main/java/com/microsoft/playwright/Page.java index ba9370b9..ab5dbfe0 100644 --- a/playwright/src/main/java/com/microsoft/playwright/Page.java +++ b/playwright/src/main/java/com/microsoft/playwright/Page.java @@ -253,13 +253,13 @@ public interface Page extends AutoCloseable { this.noWaitAfter = noWaitAfter; return this; } + public ClickOptions withPosition(double x, double y) { + return withPosition(new Position(x, y)); + } public ClickOptions withPosition(Position position) { this.position = position; return this; } - public ClickOptions withPosition(double x, double y) { - return withPosition(new Position(x, y)); - } public ClickOptions withTimeout(double timeout) { this.timeout = timeout; return this; @@ -332,13 +332,13 @@ public interface Page extends AutoCloseable { this.noWaitAfter = noWaitAfter; return this; } + public DblclickOptions withPosition(double x, double y) { + return withPosition(new Position(x, y)); + } public DblclickOptions withPosition(Position position) { this.position = position; return this; } - public DblclickOptions withPosition(double x, double y) { - return withPosition(new Position(x, y)); - } public DblclickOptions withTimeout(double timeout) { this.timeout = timeout; return this; @@ -548,13 +548,13 @@ public interface Page extends AutoCloseable { this.modifiers = modifiers; return this; } + public HoverOptions withPosition(double x, double y) { + return withPosition(new Position(x, y)); + } public HoverOptions withPosition(Position position) { this.position = position; return this; } - public HoverOptions withPosition(double x, double y) { - return withPosition(new Position(x, y)); - } public HoverOptions withTimeout(double timeout) { this.timeout = timeout; return this; @@ -861,6 +861,9 @@ public interface Page extends AutoCloseable { */ public ScreenshotType type; + public ScreenshotOptions withClip(double x, double y, double width, double height) { + return withClip(new Clip(x, y, width, height)); + } public ScreenshotOptions withClip(Clip clip) { this.clip = clip; return this; @@ -998,6 +1001,9 @@ public interface Page extends AutoCloseable { this.noWaitAfter = noWaitAfter; return this; } + public TapOptions withPosition(double x, double y) { + return withPosition(new Position(x, y)); + } public TapOptions withPosition(Position position) { this.position = position; return this; diff --git a/playwright/src/test/java/com/microsoft/playwright/TestBrowserContextProxy.java b/playwright/src/test/java/com/microsoft/playwright/TestBrowserContextProxy.java index bdf53d9e..e49e08d8 100644 --- a/playwright/src/test/java/com/microsoft/playwright/TestBrowserContextProxy.java +++ b/playwright/src/test/java/com/microsoft/playwright/TestBrowserContextProxy.java @@ -41,7 +41,7 @@ public class TestBrowserContextProxy extends TestBase { void shouldThrowForMissingGlobalProxy() { Browser browser = browserType.launch(createLaunchOptions()); try { - browser.newContext(new Browser.NewContextOptions().withProxy(new Proxy("localhost:" + server.PORT))); + browser.newContext(new Browser.NewContextOptions().withProxy("localhost:" + server.PORT)); fail("did not throw"); } catch (PlaywrightException e) { assertTrue(e.getMessage().contains("Browser needs to be launched with the global proxy")); @@ -62,8 +62,7 @@ public class TestBrowserContextProxy extends TestBase { writer.write("Served by the proxy"); } }); - BrowserContext context = browser.newContext(new Browser.NewContextOptions().withProxy( - new Proxy("localhost:" + server.PORT))); + BrowserContext context = browser.newContext(new Browser.NewContextOptions().withProxy("localhost:" + server.PORT)); Page page = context.newPage(); page.navigate("http://non-existent.com/target.html"); assertEquals("Served by the proxy", page.title()); diff --git a/playwright/src/test/java/com/microsoft/playwright/TestScreencast.java b/playwright/src/test/java/com/microsoft/playwright/TestScreencast.java index 1965864a..d1d7e866 100644 --- a/playwright/src/test/java/com/microsoft/playwright/TestScreencast.java +++ b/playwright/src/test/java/com/microsoft/playwright/TestScreencast.java @@ -16,8 +16,6 @@ package com.microsoft.playwright; -import com.microsoft.playwright.options.RecordVideoSize; -import com.microsoft.playwright.options.ViewportSize; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -30,8 +28,8 @@ public class TestScreencast extends TestBase { @Test void shouldExposeVideoPath(@TempDir Path videosDir) { BrowserContext context = browser.newContext(new Browser.NewContextOptions() - .withRecordVideoDir(videosDir).withRecordVideoSize(new RecordVideoSize(320, 240)) - .withViewportSize(new ViewportSize(320, 240))); + .withRecordVideoDir(videosDir).withRecordVideoSize(320, 240) + .withViewportSize(320, 240)); Page page = context.newPage(); page.evaluate("() => document.body.style.backgroundColor = 'red'"); Path path = page.video().path(); diff --git a/tools/api-generator/src/main/java/com/microsoft/playwright/tools/ApiGenerator.java b/tools/api-generator/src/main/java/com/microsoft/playwright/tools/ApiGenerator.java index 477e9d92..4d01c521 100644 --- a/tools/api-generator/src/main/java/com/microsoft/playwright/tools/ApiGenerator.java +++ b/tools/api-generator/src/main/java/com/microsoft/playwright/tools/ApiGenerator.java @@ -108,7 +108,6 @@ abstract class Element { // Represents return type of a method, type of a method param or type of a field. class TypeRef extends Element { String customType; - boolean isNestedClass; private static final Map customTypeNames = new HashMap<>(); static { @@ -210,7 +209,6 @@ class TypeRef extends Element { } else { customType = toTitle(parent.parent.jsonName) + toTitle(parent.jsonName); typeScope().createNestedClass(customType, this, jsonElement.getAsJsonObject()); - isNestedClass = true; } } } @@ -225,6 +223,14 @@ class TypeRef extends Element { return convertBuiltinType(stripNullable()); } + boolean isCustomClass() { + JsonObject jsonObject = stripNullable(); + if (!"Object".equals(jsonObject.get("name").getAsString())) { + return false; + } + return !jsonElement.getAsJsonObject().has("templates"); + } + boolean isTypeUnion() { if (isNullable()) { return false; @@ -406,7 +412,7 @@ class TypeRef extends Element { } abstract class TypeDefinition extends Element { - final List classes = new ArrayList<>(); + final List classes = new ArrayList<>(); static final Types types = new Types(); @@ -441,23 +447,23 @@ abstract class TypeDefinition extends Element { void createTopLevelClass(String name, Element parent, JsonObject jsonObject) { Map map = topLevelTypes(); - TypeDefinition existing = map.putIfAbsent(name, new NestedClass(parent, name, jsonObject)); - if (existing != null && (!(existing instanceof NestedClass))) { + TypeDefinition existing = map.putIfAbsent(name, new CustomClass(parent, name, jsonObject)); + if (existing != null && (!(existing instanceof CustomClass))) { throw new RuntimeException("Two classes with same name have different values:\n" + jsonObject + "\n" + existing.jsonElement); } } void createNestedClass(String name, Element parent, JsonObject jsonObject) { - for (NestedClass c : classes) { + for (CustomClass c : classes) { if (c.name.equals(name)) { return; } } - classes.add(new NestedClass(parent, name, jsonObject)); + classes.add(new CustomClass(parent, name, jsonObject)); } void writeTo(List output, String offset) { - for (NestedClass c : classes) { + for (CustomClass c : classes) { c.writeTo(output, offset); } } @@ -680,7 +686,7 @@ class Field extends Element { final String name; final TypeRef type; - Field(NestedClass parent, String name, JsonObject jsonElement) { + Field(CustomClass parent, String name, JsonObject jsonElement) { super(parent, jsonElement); this.name = name; this.type = new TypeRef(this, jsonElement.getAsJsonObject().get("type")); @@ -717,23 +723,25 @@ class Field extends Element { } return; } - if (asList("Page.click.options.position", - "Page.dblclick.options.position", - "Page.hover.options.position", - "Frame.click.options.position", - "Frame.dblclick.options.position", - "Frame.hover.options.position", - "ElementHandle.click.options.position", - "ElementHandle.dblclick.options.position", - "ElementHandle.hover.options.position").contains(jsonPath)) { - output.add(offset + "public " + parentClass + " withPosition(Position position) {"); - output.add(offset + " this.position = position;"); - output.add(offset + " return this;"); - output.add(offset + "}"); - output.add(offset + "public " + parentClass + " withPosition(double x, double y) {"); - output.add(offset + " return withPosition(new Position(x, y));"); - output.add(offset + "}"); - return; + if (type.isCustomClass()) { + TypeDefinition customType = topLevelTypes().get(type.customType); + if (customType instanceof CustomClass) { + CustomClass clazz = (CustomClass) customType; + List params = new ArrayList<>(); + List args = new ArrayList<>(); + for (Field f : clazz.fields) { + if (!f.isRequired()) { + continue; + } + params.add(f.type.toJava() + " " + f.name); + args.add(f.name); + } + if (!params.isEmpty()) { + output.add(offset + "public " + parentClass + " with" + toTitle(name) + "(" + String.join(", ", params) + ") {"); + output.add(offset + " return with" + toTitle(name) + "(new " + type.toJava() + "(" + String.join(", ", args) + "));"); + output.add(offset + "}"); + } + } } if ("Route.resume.options.postData".equals(jsonPath)) { output.add(offset + "public " + parentClass + " withPostData(String postData) {"); @@ -741,36 +749,7 @@ class Field extends Element { output.add(offset + " return this;"); output.add(offset + "}"); } - if ("ViewportSize".equals(type.toJava())) { - output.add(offset + "public " + parentClass + " with" + toTitle(name) + "(int width, int height) {"); - output.add(offset + " return with" + toTitle(name) + "(new " + type.toJava() + "(width, height));"); - output.add(offset + "}"); - } - if (name.equals("httpCredentials")) { - output.add(offset + "public " + parentClass + " with" + toTitle(name) + "(String username, String password) {"); - output.add(offset + " this." + name + " = new " + type.toJava() + "(username, password);"); - output.add(offset + " return this;"); - } else if (type.isNestedClass) { - output.add(offset + "public " + type.toJava() + " set" + toTitle(name) + "() {"); - output.add(offset + " this." + name + " = new " + type.toJava() + "();"); - output.add(offset + " return this." + name + ";"); - } else if ("Browser.VideoSize".equals(type.toJava()) || "VideoSize".equals(type.toJava())) { - output.add(offset + "public " + parentClass + " with" + toTitle(name) + "(int width, int height) {"); - output.add(offset + " this." + name + " = new " + type.toJava() + "(width, height);"); - output.add(offset + " return this;"); - } else { - String paramType = type.toJava(); - if ("Boolean".equals(paramType)) { - paramType = "boolean"; - } else if ("Integer".equals(paramType)) { - paramType = "int"; - } else if ("Double".equals(paramType)) { - paramType = "double"; - } - writeGenericBuilderMethod(output, offset, parentClass, paramType); - return; - } - output.add(offset + "}"); + writeGenericBuilderMethod(output, offset, parentClass, type.toJava()); } private void writeGenericBuilderMethod(List output, String offset, String parentClass, String paramType) { @@ -931,11 +910,11 @@ class Interface extends TypeDefinition { } } -class NestedClass extends TypeDefinition { +class CustomClass extends TypeDefinition { final String name; final List fields = new ArrayList<>(); - NestedClass(Element parent, String name, JsonObject jsonElement) { + CustomClass(Element parent, String name, JsonObject jsonElement) { super(parent, true, jsonElement); this.name = name; @@ -979,7 +958,7 @@ class NestedClass extends TypeDefinition { if (asList("RecordHar", "RecordVideo").contains(name)) { output.add("import java.nio.file.Path;"); } - String access = (parent.typeScope() instanceof NestedClass) || topLevelTypes().containsKey(name) ? "public " : ""; + String access = (parent.typeScope() instanceof CustomClass) || topLevelTypes().containsKey(name) ? "public " : ""; output.add(offset + access + "class " + name + " {"); String bodyOffset = offset + " "; super.writeTo(output, bodyOffset);