Browser handler #182

hanslovsky · 2018-07-13T15:43:36Z

As outlined in #175 it is desirable in some cases to not use the java.awt.Desktop to browse the authorizationUrl in AuthorizationCodeInstalledApp, e.g. when running a JavaFX UI. For that purpose, I created a Browser interface with a single browse(String) method that is passed as an argument to AuthorizationCodeInstalledApp. In order not to introduce any breaking changes, there is also a DefaultBrowser that simply delegates to AuthorizationCodeInstalledApp.browse.

Fixes #175

Note: mvn findbugs:check fails but it also fails on the dev branch.

Default behavior should not change with a newly introduced feature.

googlebot · 2018-07-13T15:43:39Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

hanslovsky · 2018-07-16T14:32:11Z

I signed it!

googlebot · 2018-07-16T14:32:14Z

CLAs look good, thanks!

chingor13

Thanks for this PR!

...n/java/com/google/api/client/extensions/java6/auth/oauth2/AuthorizationCodeInstalledApp.java

+    System.out.println("  " + url);
+    try {
+      browser.browse(url);
+    } catch (Exception e) {


...n/java/com/google/api/client/extensions/java6/auth/oauth2/AuthorizationCodeInstalledApp.java

+     * @param url url to browse
+     * @throws Exception
+     */
+    public void browse(String url) throws Exception;


...n/java/com/google/api/client/extensions/java6/auth/oauth2/AuthorizationCodeInstalledApp.java

-    browse(authorizationUrl.build());
+    String url = authorizationUrl.build();
+    Preconditions.checkNotNull(url);
+    System.out.println("Please open the following address in your browser:");


- Browser.browse throws IOException instead of Exception - Do not swallow exceptions but let caller deal with it - Remove superfluous println statements

chingor13

Thank you. LGTM.

We're currently blocked on new features so we can't merge this yet.

hanslovsky added 2 commits April 21, 2018 13:39

Add URL handler to let caller decide how to browse authentication URL

a42ab75

Change default browser behavior to AuthorizationCodeInstalledApp.browse

2e62bdf

Default behavior should not change with a newly introduced feature.

JustinBeckwith assigned chingor13 Jul 16, 2018

JustinBeckwith requested a review from a team July 16, 2018 15:30

chingor13 reviewed Jul 16, 2018

View reviewed changes

chingor13 mentioned this pull request Jul 16, 2018

Unable to open browser warning #174

Closed

Address comments by @chingor13 in #182

2a1cf0d

- Browser.browse throws IOException instead of Exception - Do not swallow exceptions but let caller deal with it - Remove superfluous println statements

chingor13 approved these changes Jul 17, 2018

View reviewed changes

chingor13 added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jul 17, 2018

jeanbza removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Aug 2, 2018

chingor13 merged commit f82cd2e into googleapis:dev Aug 2, 2018

google-admin unassigned chingor13 Sep 11, 2018

hanslovsky mentioned this pull request Jan 19, 2019

AuthorizationCodeInstalledApp should not use java.awt.Desktop #175

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Browser handler #182

Browser handler #182

hanslovsky commented Jul 13, 2018

googlebot commented Jul 13, 2018

hanslovsky commented Jul 16, 2018

googlebot commented Jul 16, 2018

chingor13 left a comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

chingor13 left a comment

Browser handler #182

Browser handler #182

Conversation

hanslovsky commented Jul 13, 2018

googlebot commented Jul 13, 2018

What to do if you already signed the CLA

Individual signers

Corporate signers

hanslovsky commented Jul 16, 2018

googlebot commented Jul 16, 2018

chingor13 left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

chingor13 left a comment

Choose a reason for hiding this comment