fix: clipboard get/set contentType #478

KazuCocoa · 2019-08-12T14:52:54Z

Below error happens against [debug] [WD Proxy] Proxying [POST /wd/hub/session/50c33b86-8579-4d8a-911b-b175996978b6/appium/device/set_clipboard] to [POST http://localhost:8200/session/24fe3836-7a6c-4a01-a57c-be0c4a6e327e/appium/device/set_clipboard] with body: {"contentType":"plaintext","content":"aGFwcHkgdGVzdGluZw==","label":"Note"} since the contentType was lower case. Not sure from when, but recent espresso driver can work for PLAINTEXT case even if previosly it accepts plaintext.

Selenium::WebDriver::Error::InvalidArgumentError: io.appium.espressoserver.lib.handlers.exceptions.InvalidArgumentException: java.lang.IllegalArgumentException: Parameter specified as non-null is null: method kotlin.jvm.internal.Intrinsics.checkParameterIsNotNull, parameter contentType
        at io.appium.espressoserver.lib.handlers.SetClipboard.handleInternal(SetClipboard.kt:42)
        at io.appium.espressoserver.lib.handlers.SetClipboard.handleInternal(SetClipboard.kt:32)
        at io.appium.espressoserver.lib.handlers.RequestHandler$DefaultImpls.handle(RequestHandler.kt:28)
        at io.appium.espressoserver.lib.handlers.SetClipboard.handle(SetClipboard.kt:32)
        at io.appium.espressoserver.lib.handlers.SetClipboard.handle(SetClipboard.kt:32)
        at io.appium.espressoserver.lib.http.Router.route(Router.kt:216)
        at io.appium.espressoserver.lib.http.Server.serve(Server.kt:53)
        at fi.iki.elonen.NanoHTTPD$HTTPSession.execute(NanoHTTPD.java:945)
        at fi.iki.elonen.NanoHTTPD$ClientHandler.run(NanoHTTPD.java:192)
        at java.lang.Thread.run(Thread.java:764)
Caused by: java.lang.IllegalArgumentException: Parameter specified as non-null is null: method kotlin.jvm.internal.Intrinsics.checkParameterIsNotNull, parameter contentType
        at io.appium.espressoserver.lib.handlers.SetClipboard$SetClipboardRunnable.<init>(Unknown Source:2)
        at io.appium.espressoserver.lib.handlers.SetClipboard.handleInternal(SetClipboard.kt:39)
        ... 9 more
from InvalidArgumentError: io.appium.espressoserver.lib.handlers.exceptions.InvalidArgumentException: java.lang.IllegalArgumentException: Parameter specified as non-null is null: method kotlin.jvm.internal.Intrinsics.checkParameterIsNotNull, parameter contentType

In this PR, I changed to accept such lower case, too. Uppercase and lowercase are enough so far.
contentType can be null, so I've added ?.

imurchie

Small typo in comment. Otherwise great!

imurchie · 2019-08-12T15:10:09Z

espresso-server/app/src/androidTest/java/io/appium/espressoserver/lib/handlers/GetClipboard.kt

@@ -33,6 +33,13 @@ class GetClipboard : RequestHandler<GetClipboardParams, String?> {

    @Throws(AppiumException::class)
    override fun handleInternal(params: GetClipboardParams): String? {
+        // Can be null if contentType was no plaintext


'not plaintext'?

imurchie · 2019-08-12T15:10:45Z

espresso-server/app/src/androidTest/java/io/appium/espressoserver/lib/handlers/SetClipboard.kt

@@ -35,6 +34,14 @@ class SetClipboard : RequestHandler<SetClipboardParams, Void?> {
    @Throws(AppiumException::class)
    override fun handleInternal(params: SetClipboardParams): Void? {
        params.content ?: throw InvalidArgumentException("The 'content' argument is mandatory")
+
+        // Can be null if contentType was no plaintext


mykola-mokhnach · 2019-08-12T17:21:07Z

espresso-server/app/src/androidTest/java/io/appium/espressoserver/lib/handlers/SetClipboard.kt

+        // Can be null if contentType was no plaintext
+        if (params.contentType == null
+            || !ClipboardDataType.supportedDataTypes().contains(params.contentType.toString().toLowerCase())) {
+


the empty line is redundant

...sso-server/app/src/androidTest/java/io/appium/espressoserver/lib/model/GetClipboardParams.kt

mykola-mokhnach · 2019-08-12T17:27:05Z

espresso-server/app/src/androidTest/java/io/appium/espressoserver/lib/handlers/GetClipboard.kt

@@ -33,6 +33,13 @@ class GetClipboard : RequestHandler<GetClipboardParams, String?> {

    @Throws(AppiumException::class)
    override fun handleInternal(params: GetClipboardParams): String? {
+        // Can be null if contentType was no plaintext
+        if (params.contentType == null


I assume that it should be set to 'plaintext' be default if nothing else was set

Yes, I also assumed so, but debugger showed here was null for {"contentType":"plaintext","content":"aGFwcHkgdGVzdGluZw==","label":"Note"}
Maybe, something was wrong in Gson mapping.
(would like to dig further as after this fix)

mykola-mokhnach · 2019-08-13T05:25:11Z

espresso-server/app/src/androidTest/java/io/appium/espressoserver/lib/handlers/SetClipboard.kt

@@ -35,6 +34,13 @@ class SetClipboard : RequestHandler<SetClipboardParams, Void?> {
    @Throws(AppiumException::class)
    override fun handleInternal(params: SetClipboardParams): Void? {
        params.content ?: throw InvalidArgumentException("The 'content' argument is mandatory")
+
+        // FIXME: Can be null while contentType should be ClipboardDataType.PLAINTEXT by default
+        if (params.contentType == null


so it cannot be null anymore, or it still can?

It still can if a request had no 'plaintext' or 'PLAINTEXT' like 'image' in content Type attribute

this is weird..., perhaps we just don't know how GSON works

Yeah...
When I used Gson, non-null did not work. I remember such discussion..

I found some articles about GSON such as:
https://proandroiddev.com/most-elegant-way-of-using-gson-kotlin-with-default-values-and-null-safety-b6216ac5328c

I also found a Japanese article decoding the generated code into Java.
According to them, we cannot handle properly for non-null (default) value in some case in Kotlin.
So, we should do like https://github.com/appium/appium-espresso-driver/pull/478/files#diff-88e41c77db8f06054ff5887d7d6e3031R27

It worked fine.

error example:

Selenium::WebDriver::Error::InvalidArgumentError: io.appium.espressoserver.lib.handlers.exceptions.InvalidArgumentException: Only [plaintext] content types are supported. 'image' is given instead at io.appium.espressoserver.lib.model.ClipboardDataType$Companion.invalidClipboardDataType(ClipboardDataType.kt:14) at io.appium.espressoserver.lib.model.SetClipboardParams.getContentType(SetClipboardParams.kt:33) at io.appium.espressoserver.lib.handlers.SetClipboard.handleInternal(SetClipboard.kt:40) at io.appium.espressoserver.lib.handlers.SetClipboard.handleInternal(SetClipboard.kt:31) at io.appium.espressoserver.lib.handlers.RequestHandler$DefaultImpls.handle(RequestHandler.kt:28) at io.appium.espressoserver.lib.handlers.SetClipboard.handle(SetClipboard.kt:31) at io.appium.espressoserver.lib.handlers.SetClipboard.handle(SetClipboard.kt:31) at io.appium.espressoserver.lib.http.Router.route(Router.kt:216) at io.appium.espressoserver.lib.http.Server.serve(Server.kt:53) at fi.iki.elonen.NanoHTTPD$HTTPSession.execute(NanoHTTPD.java:945) at fi.iki.elonen.NanoHTTPD$ClientHandler.run(NanoHTTPD.java:192) at java.lang.Thread.run(Thread.java:764) from InvalidArgumentError: io.appium.espressoserver.lib.han

mykola-mokhnach · 2019-08-13T05:38:43Z

The test now fails :(

KazuCocoa · 2019-08-13T05:34:37Z

test/functional/commands/clipboard-e2e-specs.js

+  it('should set and get clipboard', async function () {
+    await driver.setClipboard(new Buffer.from('Hello').toString('base64'), 'plaintext');
+    await driver.getClipboard().should.eventually.eql('Hello');
+  });


Note.
Expects 'SGVsbG8=' instead of Hello

KazuCocoa · 2019-08-13T05:42:08Z

Yes. Will fix it later

KazuCocoa · 2019-08-13T14:49:14Z

test/functional/commands/clipboard-e2e-specs.js

+    await driver.setClipboard(new Buffer.from('Hello').toString('base64'), 'plaintext');
+    // 'SGVsbG8=' is 'Hello' in base 64 encoding with a new line.
+    const text = await driver.getClipboard('PLAINTEXT');
+    text.should.eql('SGVsbG8=\n');


This behaviour, '\n', does not change from UIA2

mykola-mokhnach · 2019-08-13T17:08:35Z

...sso-server/app/src/androidTest/java/io/appium/espressoserver/lib/model/SetClipboardParams.kt

 data class SetClipboardParams(
-    val contentType: ClipboardDataType = ClipboardDataType.PLAINTEXT,
+    @SerializedName("contentType")
+    private val _contentType: String?,


what the point in having the underscore in the property name?

To avoid to conflict with val contentType : ClipboardDataType in line 26

mykola-mokhnach · 2019-08-13T17:33:35Z

...sso-server/app/src/androidTest/java/io/appium/espressoserver/lib/model/SetClipboardParams.kt

+) : AppiumParams() {
+    val contentType : ClipboardDataType
+        get() {
+            if (_contentType == null) return ClipboardDataType.PLAINTEXT


can we move the getter to a shared method to avoid duplication?

KazuCocoa added 2 commits August 12, 2019 23:32

allow plaintext as compatibility

b2c903d

add tests for clipoard

fa77844

KazuCocoa mentioned this pull request Aug 12, 2019

Add tests for espresso driver appium/ruby_lib_core#235

Merged

imurchie approved these changes Aug 12, 2019

View reviewed changes

mykola-mokhnach reviewed Aug 12, 2019

View reviewed changes

...sso-server/app/src/androidTest/java/io/appium/espressoserver/lib/model/GetClipboardParams.kt Outdated Show resolved Hide resolved

mykola-mokhnach reviewed Aug 12, 2019

View reviewed changes

KazuCocoa added 2 commits August 13, 2019 10:13

fix comments

f9cf571

stay requring non-null

7b0ecb5

mykola-mokhnach reviewed Aug 13, 2019

View reviewed changes

KazuCocoa commented Aug 13, 2019

View reviewed changes

fix GSON parser, text case

fc9cf53

KazuCocoa mentioned this pull request Aug 13, 2019

Fix GSON default value in Kotlin #480

Open

KazuCocoa added 5 commits August 13, 2019 21:20

remove a redundant line

17cde48

turn companion object in clipboarddatatype

d3fd61a

turn control flow

ff791f3

remove redundant toUpperCase

f0a6c15

fix e2e

1b63d7c

KazuCocoa commented Aug 13, 2019

View reviewed changes

mykola-mokhnach reviewed Aug 13, 2019

View reviewed changes

mykola-mokhnach approved these changes Aug 13, 2019

View reviewed changes

mykola-mokhnach reviewed Aug 13, 2019

View reviewed changes

turn get contentType

68756da

KazuCocoa merged commit 7d1e981 into appium:master Aug 14, 2019

KazuCocoa deleted the km/fix-clipboard branch August 14, 2019 01:37

KazuCocoa mentioned this pull request Aug 28, 2019

Can't use setClipboard in espresso #493

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: clipboard get/set contentType #478

fix: clipboard get/set contentType #478

KazuCocoa commented Aug 12, 2019

imurchie left a comment

imurchie Aug 12, 2019

imurchie Aug 12, 2019

mykola-mokhnach Aug 12, 2019

mykola-mokhnach Aug 12, 2019

KazuCocoa Aug 13, 2019

mykola-mokhnach Aug 13, 2019

KazuCocoa Aug 13, 2019

mykola-mokhnach Aug 13, 2019

KazuCocoa Aug 13, 2019

KazuCocoa Aug 13, 2019 •

edited

Loading

mykola-mokhnach commented Aug 13, 2019

KazuCocoa Aug 13, 2019

KazuCocoa commented Aug 13, 2019

KazuCocoa Aug 13, 2019

mykola-mokhnach Aug 13, 2019

KazuCocoa Aug 13, 2019

mykola-mokhnach Aug 13, 2019

fix: clipboard get/set contentType #478

fix: clipboard get/set contentType #478

Conversation

KazuCocoa commented Aug 12, 2019

imurchie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KazuCocoa Aug 13, 2019 • edited Loading

Choose a reason for hiding this comment

mykola-mokhnach commented Aug 13, 2019

Choose a reason for hiding this comment

KazuCocoa commented Aug 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KazuCocoa Aug 13, 2019 •

edited

Loading