Update all samples for 1.10

This CL tests and fixes samples for the upcoming 1.10 release. Everything is now working with 1.10

[mlinder, 2012-06-08 20:15:52]

Updated samples; not yet fully done though. Sorry for the huge file count :)

[rmistry, 2012-06-11 12:28:50]

http://codereview.appspot.com/6297067/diff/1/adexchangebuyer-cmdline-sample/p...
File adexchangebuyer-cmdline-sample/pom.xml (right):

http://codereview.appspot.com/6297067/diff/1/adexchangebuyer-cmdline-sample/p...
adexchangebuyer-cmdline-sample/pom.xml:18: <!--    
<url>http://code.google.com/p/google-api-java-client/issues/list</url> -->
Why comment this out?

http://codereview.appspot.com/6297067/diff/1/adexchangebuyer-cmdline-sample/p...
adexchangebuyer-cmdline-sample/pom.xml:134: <version>1.3.0</version>
I think its a good idea to update the shared-sample-cmdline version when we
release. This is something I should have done in the past releases.

http://codereview.appspot.com/6297067/diff/1/adsense-cmdline-sample/src/main/...
File
adsense-cmdline-sample/src/main/java/com/google/api/services/samples/adsense/cmdline/AdSenseSample.java
(right):

http://codereview.appspot.com/6297067/diff/1/adsense-cmdline-sample/src/main/...
adsense-cmdline-sample/src/main/java/com/google/api/services/samples/adsense/cmdline/AdSenseSample.java:22:
import com.google.api.services.adsense.v1_1.Adsense;
This is interesting, I do not see this package path when I generated adsense v1.
How did you generate it?

http://codereview.appspot.com/6297067/diff/1/calendar-cmdline-sample/src/main...
File
calendar-cmdline-sample/src/main/java/com/google/api/services/samples/calendar/cmdline/CalendarSample.java
(right):

http://codereview.appspot.com/6297067/diff/1/calendar-cmdline-sample/src/main...
calendar-cmdline-sample/src/main/java/com/google/api/services/samples/calendar/cmdline/CalendarSample.java:102:
@Override
Extra space before @Override here and below.

http://codereview.appspot.com/6297067/diff/1/moderator-cmdline-sample/src/mai...
File
moderator-cmdline-sample/src/main/java/com/google/api/services/samples/moderator/cmdline/ModeratorSample.java
(right):

http://codereview.appspot.com/6297067/diff/1/moderator-cmdline-sample/src/mai...
moderator-cmdline-sample/src/main/java/com/google/api/services/samples/moderator/cmdline/ModeratorSample.java:67:
long seriesId = series.getId().getSeriesId();
why not cast it to (int) here to avoid doing it in multiple places below? Same
for topicId and submissionId.

http://codereview.appspot.com/6297067/diff/1/shared/shared-sample-appengine/p...
File shared/shared-sample-appengine/pom.xml (right):

http://codereview.appspot.com/6297067/diff/1/shared/shared-sample-appengine/p...
shared/shared-sample-appengine/pom.xml:11: <version>1.3.0</version>
Why not go to 1.2.0 here?

http://codereview.appspot.com/6297067/diff/1/shared/shared-sample-cmdline/src...
File
shared/shared-sample-cmdline/src/main/java/com/google/api/services/samples/shared/cmdline/oauth2/OAuth2Native.java
(right):

http://codereview.appspot.com/6297067/diff/1/shared/shared-sample-cmdline/src...
shared/shared-sample-cmdline/src/main/java/com/google/api/services/samples/shared/cmdline/oauth2/OAuth2Native.java:117:
transport, jsonFactory, clientSecrets, scopes).setAccessType("offline").build();
Should we add .setApprovalPrompt("force")?

http://codereview.appspot.com/6297067/diff/1/youtube-jsonc-sample/src/main/ja...
File
youtube-jsonc-sample/src/main/java/com/google/api/client/sample/youtube/YouTubeClient.java
(right):

http://codereview.appspot.com/6297067/diff/1/youtube-jsonc-sample/src/main/ja...
youtube-jsonc-sample/src/main/java/com/google/api/client/sample/youtube/YouTubeClient.java:44:
@Override
Extra space.

[mlinder, 2012-06-11 15:22:35]

Fixed some of the issues, and commented on the other ones. Your opinion is
required

http://codereview.appspot.com/6297067/diff/1/adexchangebuyer-cmdline-sample/p...
File adexchangebuyer-cmdline-sample/pom.xml (right):

http://codereview.appspot.com/6297067/diff/1/adexchangebuyer-cmdline-sample/p...
adexchangebuyer-cmdline-sample/pom.xml:18: <!--    
<url>http://code.google.com/p/google-api-java-client/issues/list</url> -->
evil keyboard shortcuts from outer space attacking my eclipse environment.

(reverted.)

On 2012/06/11 12:28:50, rmistry wrote:
> Why comment this out?

http://codereview.appspot.com/6297067/diff/1/adexchangebuyer-cmdline-sample/p...
adexchangebuyer-cmdline-sample/pom.xml:134: <version>1.3.0</version>
Yup. One suggestion: Would it be a good idea to skip 7/8 subversions and skip to
1.10 directly so that the shared-* version matches the api version?

On 2012/06/11 12:28:50, rmistry wrote:
> I think its a good idea to update the shared-sample-cmdline version when we
> release. This is something I should have done in the past releases.

http://codereview.appspot.com/6297067/diff/1/adsense-cmdline-sample/src/main/...
File
adsense-cmdline-sample/src/main/java/com/google/api/services/samples/adsense/cmdline/AdSenseSample.java
(right):

http://codereview.appspot.com/6297067/diff/1/adsense-cmdline-sample/src/main/...
adsense-cmdline-sample/src/main/java/com/google/api/services/samples/adsense/cmdline/AdSenseSample.java:22:
import com.google.api.services.adsense.v1_1.Adsense;
Hm. Interesting. Using the .sh script it won't generate these. reverted

On 2012/06/11 12:28:50, rmistry wrote:
> This is interesting, I do not see this package path when I generated adsense
v1.
> How did you generate it?

http://codereview.appspot.com/6297067/diff/1/calendar-cmdline-sample/src/main...
File
calendar-cmdline-sample/src/main/java/com/google/api/services/samples/calendar/cmdline/CalendarSample.java
(right):

http://codereview.appspot.com/6297067/diff/1/calendar-cmdline-sample/src/main...
calendar-cmdline-sample/src/main/java/com/google/api/services/samples/calendar/cmdline/CalendarSample.java:102:
@Override
Not sure why, but eclipse keeps moving it. Reverted.

On 2012/06/11 12:28:50, rmistry wrote:
> Extra space before @Override here and below.

http://codereview.appspot.com/6297067/diff/1/moderator-cmdline-sample/src/mai...
File
moderator-cmdline-sample/src/main/java/com/google/api/services/samples/moderator/cmdline/ModeratorSample.java
(right):

http://codereview.appspot.com/6297067/diff/1/moderator-cmdline-sample/src/mai...
moderator-cmdline-sample/src/main/java/com/google/api/services/samples/moderator/cmdline/ModeratorSample.java:67:
long seriesId = series.getId().getSeriesId();
On 2012/06/11 12:28:50, rmistry wrote:
> why not cast it to (int) here to avoid doing it in multiple places below? Same
> for topicId and submissionId.

Done.

http://codereview.appspot.com/6297067/diff/1/shared/shared-sample-appengine/p...
File shared/shared-sample-appengine/pom.xml (right):

http://codereview.appspot.com/6297067/diff/1/shared/shared-sample-appengine/p...
shared/shared-sample-appengine/pom.xml:11: <version>1.3.0</version>
I wanted to sync up the shared-* projects. I figured it would be confusing to us
(and the user) if the one project is at 1.2, and the other one is at 1.3.

On 2012/06/11 12:28:50, rmistry wrote:
> Why not go to 1.2.0 here?

http://codereview.appspot.com/6297067/diff/1/shared/shared-sample-cmdline/src...
File
shared/shared-sample-cmdline/src/main/java/com/google/api/services/samples/shared/cmdline/oauth2/OAuth2Native.java
(right):

http://codereview.appspot.com/6297067/diff/1/shared/shared-sample-cmdline/src...
shared/shared-sample-cmdline/src/main/java/com/google/api/services/samples/shared/cmdline/oauth2/OAuth2Native.java:117:
transport, jsonFactory, clientSecrets, scopes).setAccessType("offline").build();
I wouldn't as I don't see any advantage to it. If we already request an offline
token, the user shouldn't have to re-enter it every time. Would be really
annoying for the user. 

I'd either go with online-only tokens (advantage: no tokens can be leaked, ...),
or which offline stored tokens (advantage: comfort), but not offline forced
ones.

Yaniv, Ravi, your opinion on this?

On 2012/06/11 12:28:50, rmistry wrote:
> Should we add .setApprovalPrompt("force")?

http://codereview.appspot.com/6297067/diff/1/youtube-jsonc-sample/src/main/ja...
File
youtube-jsonc-sample/src/main/java/com/google/api/client/sample/youtube/YouTubeClient.java
(right):

http://codereview.appspot.com/6297067/diff/1/youtube-jsonc-sample/src/main/ja...
youtube-jsonc-sample/src/main/java/com/google/api/client/sample/youtube/YouTubeClient.java:44:
@Override
On 2012/06/11 12:28:50, rmistry wrote:
> Extra space.

Done.

[rmistry, 2012-06-11 18:01:07]

http://codereview.appspot.com/6297067/diff/1/adexchangebuyer-cmdline-sample/p...
File adexchangebuyer-cmdline-sample/pom.xml (right):

http://codereview.appspot.com/6297067/diff/1/adexchangebuyer-cmdline-sample/p...
adexchangebuyer-cmdline-sample/pom.xml:134: <version>1.3.0</version>
On 2012/06/11 15:22:35, Matthias wrote:
> Yup. One suggestion: Would it be a good idea to skip 7/8 subversions and skip
to
> 1.10 directly so that the shared-* version matches the api version?
> 

I do not think it helps to match the base library version. If we completely
overhaul shared-sample-cmdline then we need to upversion it, but if we tie it
with the base library version then we cannot do that. I recommend letting it
continue with its own versioning.

> On 2012/06/11 12:28:50, rmistry wrote:
> > I think its a good idea to update the shared-sample-cmdline version when we
> > release. This is something I should have done in the past releases.
>

http://codereview.appspot.com/6297067/diff/1/moderator-cmdline-sample/src/mai...
File
moderator-cmdline-sample/src/main/java/com/google/api/services/samples/moderator/cmdline/ModeratorSample.java
(right):

http://codereview.appspot.com/6297067/diff/1/moderator-cmdline-sample/src/mai...
moderator-cmdline-sample/src/main/java/com/google/api/services/samples/moderator/cmdline/ModeratorSample.java:67:
long seriesId = series.getId().getSeriesId();
On 2012/06/11 15:22:35, Matthias wrote:
> On 2012/06/11 12:28:50, rmistry wrote:
> > why not cast it to (int) here to avoid doing it in multiple places below?
Same
> > for topicId and submissionId.
> 
> Done.

The double cast looks strange. What about getSeriesId().intValue() ?

http://codereview.appspot.com/6297067/diff/1/shared/shared-sample-cmdline/src...
File
shared/shared-sample-cmdline/src/main/java/com/google/api/services/samples/shared/cmdline/oauth2/OAuth2Native.java
(right):

http://codereview.appspot.com/6297067/diff/1/shared/shared-sample-cmdline/src...
shared/shared-sample-cmdline/src/main/java/com/google/api/services/samples/shared/cmdline/oauth2/OAuth2Native.java:117:
transport, jsonFactory, clientSecrets, scopes).setAccessType("offline").build();
On 2012/06/11 15:22:35, Matthias wrote:
> I wouldn't as I don't see any advantage to it. If we already request an
offline
> token, the user shouldn't have to re-enter it every time. Would be really
> annoying for the user. 
> 
> I'd either go with online-only tokens (advantage: no tokens can be leaked,
...),
> or which offline stored tokens (advantage: comfort), but not offline forced
> ones.
> 
> Yaniv, Ravi, your opinion on this?
> 

I do not feel strongly either way. I will let Yaniv comment on this.

> On 2012/06/11 12:28:50, rmistry wrote:
> > Should we add .setApprovalPrompt("force")?
>

[rmistry, 2012-06-11 18:14:46]

http://codereview.appspot.com/6297067/diff/4003/bigquery-appengine-sample/pom...
File bigquery-appengine-sample/pom.xml (right):

http://codereview.appspot.com/6297067/diff/4003/bigquery-appengine-sample/pom...
bigquery-appengine-sample/pom.xml:70: <pluginManagement>
Out of curiousity were you able to successfully run this sample?

Also, in the instructions.html there is a missing line at the end could you
please complete it. Thanks!

[mlinder, 2012-06-11 20:07:45]

All done!

http://codereview.appspot.com/6297067/diff/1/moderator-cmdline-sample/src/mai...
File
moderator-cmdline-sample/src/main/java/com/google/api/services/samples/moderator/cmdline/ModeratorSample.java
(right):

http://codereview.appspot.com/6297067/diff/1/moderator-cmdline-sample/src/mai...
moderator-cmdline-sample/src/main/java/com/google/api/services/samples/moderator/cmdline/ModeratorSample.java:67:
long seriesId = series.getId().getSeriesId();
On 2012/06/11 18:01:08, rmistry wrote:
> On 2012/06/11 15:22:35, Matthias wrote:
> > On 2012/06/11 12:28:50, rmistry wrote:
> > > why not cast it to (int) here to avoid doing it in multiple places below?
> Same
> > > for topicId and submissionId.
> > 
> > Done.
> 
> The double cast looks strange. What about getSeriesId().intValue() ?

Done. (Thanks, didn't know about that :))

http://codereview.appspot.com/6297067/diff/4003/bigquery-appengine-sample/pom...
File bigquery-appengine-sample/pom.xml (right):

http://codereview.appspot.com/6297067/diff/4003/bigquery-appengine-sample/pom...
bigquery-appengine-sample/pom.xml:70: <pluginManagement>
No, this is the outdated dashboard sample. I've started fixing it, but there is
way too much outdated stuff in here right now. Left it for later as it is
outdated.

On 2012/06/11 18:14:46, rmistry wrote:
> Out of curiousity were you able to successfully run this sample?
> 
> Also, in the instructions.html there is a missing line at the end could you
> please complete it. Thanks!

[rmistry, 2012-06-11 20:17:48]

http://codereview.appspot.com/6297067/diff/68/bigquery-appengine-sample/instr...
File bigquery-appengine-sample/instructions.html (right):

http://codereview.appspot.com/6297067/diff/68/bigquery-appengine-sample/instr...
bigquery-appengine-sample/instructions.html:1: <html>
It may be a good idea to ask lparkinson@google.com to review your changes to
bigquery-appengine-sample as well.

http://codereview.appspot.com/6297067/diff/68/bigquery-appengine-sample/instr...
bigquery-appengine-sample/instructions.html:64: <li>I get a 404 when I run my
query.  What could be wrong?<br/>Make sure you've added your Bigquery project ID
to the appengine-web.xml.</li>
Maybe say API console project ID here instead.

http://codereview.appspot.com/6297067/diff/68/oauth2-cmdline-sample/src/main/...
File
oauth2-cmdline-sample/src/main/java/com/google/api/services/samples/oauth2/cmdline/OAuth2Sample.java
(right):

http://codereview.appspot.com/6297067/diff/68/oauth2-cmdline-sample/src/main/...
oauth2-cmdline-sample/src/main/java/com/google/api/services/samples/oauth2/cmdline/OAuth2Sample.java:62:
oauth2 = new Oauth2.Builder(HTTP_TRANSPORT, JSON_FACTORY,
credential).setApplicationName(
I just checked in the CL to make this work. Can you please try it again after
regenerating the library.

[mlinder, 2012-06-11 21:00:24]

All the samples are working with 1.10.1 now. 

Laura, could you have a look at the updated version of your dashboard/BigQuery
sample, and check whether you are ok with it? Thank you.

http://codereview.appspot.com/6297067/diff/68/oauth2-cmdline-sample/src/main/...
File
oauth2-cmdline-sample/src/main/java/com/google/api/services/samples/oauth2/cmdline/OAuth2Sample.java
(right):

http://codereview.appspot.com/6297067/diff/68/oauth2-cmdline-sample/src/main/...
oauth2-cmdline-sample/src/main/java/com/google/api/services/samples/oauth2/cmdline/OAuth2Sample.java:62:
oauth2 = new Oauth2.Builder(HTTP_TRANSPORT, JSON_FACTORY,
credential).setApplicationName(
On 2012/06/11 20:17:48, rmistry wrote:
> I just checked in the CL to make this work. Can you please try it again after
> regenerating the library.

Done. Works now, thanks!

[rmistry, 2012-06-11 21:05:56]

LGTM

should also wait for LGTMs from Laura and Yaniv

[lparkinson, 2012-06-11 21:21:44]

Thanks for doing this! Just a couple comments.

http://codereview.appspot.com/6297067/diff/5122/bigquery-appengine-sample/src...
File
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/MainServlet.java
(left):

http://codereview.appspot.com/6297067/diff/5122/bigquery-appengine-sample/src...
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/MainServlet.java:63:
}
What happens if the credentials are bad (e.g. revoked by the user)?

http://codereview.appspot.com/6297067/diff/5122/bigquery-appengine-sample/src...
File
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/TaskServlet.java
(left):

http://codereview.appspot.com/6297067/diff/5122/bigquery-appengine-sample/src...
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/TaskServlet.java:74:
// If bad credentials were the problem, clear them and ask the user to refresh.
Also here, what happens if the credentials are bad?

http://codereview.appspot.com/6297067/diff/5122/bigquery-appengine-sample/src...
File
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/TaskServlet.java
(right):

http://codereview.appspot.com/6297067/diff/5122/bigquery-appengine-sample/src...
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/TaskServlet.java:14:
* out of the App Engine task queue. It gets the status of the query from
Bigquery and: - copies the
Can you reformat this, please? And update to match current behavior.

http://codereview.appspot.com/6297067/diff/5122/bigquery-appengine-sample/src...
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/TaskServlet.java:19:
*  Note that because of the auth-constraint defined in web.xml, this can only be
called by App
Extra space.

http://codereview.appspot.com/6297067/diff/5122/bigquery-appengine-sample/src...
File
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/Utils.java
(right):

http://codereview.appspot.com/6297067/diff/5122/bigquery-appengine-sample/src...
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/Utils.java:39:
class Utils {
Can you name this something that makes it clear why it's different from
BigqueryUtils and DatastoreUtils?

[yanivi, 2012-06-11 22:18:22]

Matthias, would you kindly also update the calendar & tasks android samples?

http://codereview.appspot.com/6297067/diff/5122/adexchangebuyer-cmdline-sampl...
File adexchangebuyer-cmdline-sample/pom.xml (right):

http://codereview.appspot.com/6297067/diff/5122/adexchangebuyer-cmdline-sampl...
adexchangebuyer-cmdline-sample/pom.xml:129:
<version>v1-rev20120227-1.7.0-beta</version>
my shell script had trouble with this one.  can you show me how you generated
this?

http://codereview.appspot.com/6297067/diff/5122/analytics-cmdline-sample/pom.xml
File analytics-cmdline-sample/pom.xml (right):

http://codereview.appspot.com/6297067/diff/5122/analytics-cmdline-sample/pom....
analytics-cmdline-sample/pom.xml:129:
<version>v3-revsnapshot-1.7.0-beta</version>
my shell script had trouble with this one too.

http://codereview.appspot.com/6297067/diff/5122/shared/shared-sample-cmdline/...
File
shared/shared-sample-cmdline/src/main/java/com/google/api/services/samples/shared/cmdline/oauth2/OAuth2Native.java
(right):

http://codereview.appspot.com/6297067/diff/5122/shared/shared-sample-cmdline/...
shared/shared-sample-cmdline/src/main/java/com/google/api/services/samples/shared/cmdline/oauth2/OAuth2Native.java:116:
GoogleAuthorizationCodeFlow flow = new GoogleAuthorizationCodeFlow.Builder(
Please setAccessType("online").setApprovalPrompt("auto")

Please also put a TODO for 1.11.0-beta to use
setAccessType("offline").setApprovalPrompt("force") with FileCredentialStore. 
Though we need to check what the defaults are for installed applications, e.g.
is access type offline by default & is approval_prompt force by default.

[mlinder, 2012-06-12 14:53:53]

Fixed yaniv's comments and updated the documentation for the dashboard sample.
Android samples are still a TODO

http://codereview.appspot.com/6297067/diff/5122/adexchangebuyer-cmdline-sampl...
File adexchangebuyer-cmdline-sample/pom.xml (right):

http://codereview.appspot.com/6297067/diff/5122/adexchangebuyer-cmdline-sampl...
adexchangebuyer-cmdline-sample/pom.xml:129:
<version>v1-rev20120227-1.7.0-beta</version>
Works fine with your build script!?

On 2012/06/11 22:18:23, yanivi wrote:
> my shell script had trouble with this one.  can you show me how you generated
> this?

http://codereview.appspot.com/6297067/diff/5122/analytics-cmdline-sample/pom.xml
File analytics-cmdline-sample/pom.xml (right):

http://codereview.appspot.com/6297067/diff/5122/analytics-cmdline-sample/pom....
analytics-cmdline-sample/pom.xml:129:
<version>v3-revsnapshot-1.7.0-beta</version>
Updated so it uses your build script

On 2012/06/11 22:18:23, yanivi wrote:
> my shell script had trouble with this one too.

http://codereview.appspot.com/6297067/diff/5122/bigquery-appengine-sample/src...
File
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/MainServlet.java
(left):

http://codereview.appspot.com/6297067/diff/5122/bigquery-appengine-sample/src...
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/MainServlet.java:63:
}
If the access token gets outdated, the GoogleCodeAuthorizationFlow will
automatically fetch a new Access Token. In case the user revokes the refresh
token, we will just show the "Encountered an exception (...): Forbidden / Access
denied .." message as there is not much we can do it about it.

On 2012/06/11 21:21:44, lparkinson wrote:
> What happens if the credentials are bad (e.g. revoked by the user)?

http://codereview.appspot.com/6297067/diff/5122/bigquery-appengine-sample/src...
File
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/TaskServlet.java
(left):

http://codereview.appspot.com/6297067/diff/5122/bigquery-appengine-sample/src...
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/TaskServlet.java:74:
// If bad credentials were the problem, clear them and ask the user to refresh.
When a refresh token is available, the access token will be refetched. Otherwise
the exception message will be shown to the user.

On 2012/06/11 21:21:44, lparkinson wrote:
> Also here, what happens if the credentials are bad?

http://codereview.appspot.com/6297067/diff/5122/bigquery-appengine-sample/src...
File
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/TaskServlet.java
(right):

http://codereview.appspot.com/6297067/diff/5122/bigquery-appengine-sample/src...
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/TaskServlet.java:14:
* out of the App Engine task queue. It gets the status of the query from
Bigquery and: - copies the
On 2012/06/11 21:21:44, lparkinson wrote:
> Can you reformat this, please? And update to match current behavior.

Done.

http://codereview.appspot.com/6297067/diff/5122/bigquery-appengine-sample/src...
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/TaskServlet.java:19:
*  Note that because of the auth-constraint defined in web.xml, this can only be
called by App
On 2012/06/11 21:21:44, lparkinson wrote:
> Extra space.

Done.

http://codereview.appspot.com/6297067/diff/5122/bigquery-appengine-sample/src...
File
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/Utils.java
(right):

http://codereview.appspot.com/6297067/diff/5122/bigquery-appengine-sample/src...
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/Utils.java:39:
class Utils {
On 2012/06/11 21:21:44, lparkinson wrote:
> Can you name this something that makes it clear why it's different from
> BigqueryUtils and DatastoreUtils?

Done.

http://codereview.appspot.com/6297067/diff/5122/shared/shared-sample-cmdline/...
File
shared/shared-sample-cmdline/src/main/java/com/google/api/services/samples/shared/cmdline/oauth2/OAuth2Native.java
(right):

http://codereview.appspot.com/6297067/diff/5122/shared/shared-sample-cmdline/...
shared/shared-sample-cmdline/src/main/java/com/google/api/services/samples/shared/cmdline/oauth2/OAuth2Native.java:116:
GoogleAuthorizationCodeFlow flow = new GoogleAuthorizationCodeFlow.Builder(
On 2012/06/11 22:18:23, yanivi wrote:
> Please setAccessType("online").setApprovalPrompt("auto")
> 
> Please also put a TODO for 1.11.0-beta to use
> setAccessType("offline").setApprovalPrompt("force") with FileCredentialStore. 
> Though we need to check what the defaults are for installed applications, e.g.
> is access type offline by default & is approval_prompt force by default.

Done.

[lparkinson, 2012-06-12 16:33:47]

http://codereview.appspot.com/6297067/diff/5122/bigquery-appengine-sample/src...
File
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/MainServlet.java
(left):

http://codereview.appspot.com/6297067/diff/5122/bigquery-appengine-sample/src...
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/MainServlet.java:63:
}
While it is impossible retrieve any data with bad tokens, the tokens should
still be cleared so that if the user refreshes or comes back, they'll be
presented with the normal flow rather than being permanently locked out.

On 2012/06/12 14:53:53, Matthias wrote:
> If the access token gets outdated, the GoogleCodeAuthorizationFlow will
> automatically fetch a new Access Token. In case the user revokes the refresh
> token, we will just show the "Encountered an exception (...): Forbidden /
Access
> denied .." message as there is not much we can do it about it.
> 
> On 2012/06/11 21:21:44, lparkinson wrote:
> > What happens if the credentials are bad (e.g. revoked by the user)?
>

[yanivi, 2012-06-12 16:50:08]

LGTM

wait for LGTM from Laura

you may update the Android samples in a separate changeset

[mlinder, 2012-06-12 18:13:03]

Re-added the code for error handling the case where the user revokes his refresh
token.

[lparkinson, 2012-06-12 18:59:13]

http://codereview.appspot.com/6297067/diff/10006/bigquery-appengine-sample/sr...
File
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/BigqueryUtils.java
(right):

http://codereview.appspot.com/6297067/diff/10006/bigquery-appengine-sample/sr...
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/BigqueryUtils.java:156:
String[] south = {"OK",
Nit: can you make these consistent, please?

http://codereview.appspot.com/6297067/diff/10006/bigquery-appengine-sample/sr...
File
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/MainServlet.java
(right):

http://codereview.appspot.com/6297067/diff/10006/bigquery-appengine-sample/sr...
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/MainServlet.java:60:
if (ex.getStatusCode() == 401) { // Unauthorized
Nit here also, HttpServletResponse.SC_UNAUTHORIZED might be nicer.

http://codereview.appspot.com/6297067/diff/10006/bigquery-appengine-sample/sr...
File
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/TaskServlet.java
(left):

http://codereview.appspot.com/6297067/diff/10006/bigquery-appengine-sample/sr...
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/TaskServlet.java:75:
AuthUtils authUtils = new AuthUtils(userId, transport, jsonFactory);
Can you please also re-add the unauthorized handling here? Nit: it might be
preferable to use HttpServletResponse.SC_UNAUTHORIZED.

[mlinder, 2012-06-12 20:28:26]

Removed all the small nits. Sorry for that

http://codereview.appspot.com/6297067/diff/10006/bigquery-appengine-sample/sr...
File
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/BigqueryUtils.java
(right):

http://codereview.appspot.com/6297067/diff/10006/bigquery-appengine-sample/sr...
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/BigqueryUtils.java:156:
String[] south = {"OK",
On 2012/06/12 18:59:13, lparkinson wrote:
> Nit: can you make these consistent, please?

Done.

http://codereview.appspot.com/6297067/diff/10006/bigquery-appengine-sample/sr...
File
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/MainServlet.java
(right):

http://codereview.appspot.com/6297067/diff/10006/bigquery-appengine-sample/sr...
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/MainServlet.java:60:
if (ex.getStatusCode() == 401) { // Unauthorized
Sounds good :)

On 2012/06/12 18:59:13, lparkinson wrote:
> Nit here also, HttpServletResponse.SC_UNAUTHORIZED might be nicer.

http://codereview.appspot.com/6297067/diff/10006/bigquery-appengine-sample/sr...
File
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/TaskServlet.java
(left):

http://codereview.appspot.com/6297067/diff/10006/bigquery-appengine-sample/sr...
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/TaskServlet.java:75:
AuthUtils authUtils = new AuthUtils(userId, transport, jsonFactory);
Doesn't really seem to be used, but ok.

On 2012/06/12 18:59:13, lparkinson wrote:
> Can you please also re-add the unauthorized handling here? Nit: it might be
> preferable to use HttpServletResponse.SC_UNAUTHORIZED.

[lparkinson, 2012-06-12 20:56:33]

LGTM

Thanks for doing this update!

http://codereview.appspot.com/6297067/diff/10006/bigquery-appengine-sample/sr...
File
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/TaskServlet.java
(left):

http://codereview.appspot.com/6297067/diff/10006/bigquery-appengine-sample/sr...
bigquery-appengine-sample/src/main/java/com/google/api/client/sample/bigquery/appengine/dashboard/TaskServlet.java:75:
AuthUtils authUtils = new AuthUtils(userId, transport, jsonFactory);
So it's a bit of a corner case, but if the user unauthorizes the app while the
query is running, it can manifest here. The user should never be locked out
because of stored bad credentials, so they also need to be cleared here. Nit: no
need to log at the end - message will be stored.

On 2012/06/12 20:28:26, Matthias wrote:
> Doesn't really seem to be used, but ok.
> 
> On 2012/06/12 18:59:13, lparkinson wrote:
> > Can you please also re-add the unauthorized handling here? Nit: it might be
> > preferable to use HttpServletResponse.SC_UNAUTHORIZED.
>

[mlinder, 2012-06-14 15:03:51]

Updated the versions in all pom.xml with the officially released libs. Yaniv,
Ravi, could you be so kind and have a short look at whether everything in here
is ok? 

Android is in a separate CL.

[rmistry, 2012-06-14 15:24:10]

LGTM

[mlinder, 2012-06-14 16:02:04]

submitted

[plumber1.MA, 2013-06-23 00:12:34]

Some one has hacked my phone restore asap

https://codereview.appspot.com/6297067/diff/1/adsense-cmdline-sample/src/main...
File
adsense-cmdline-sample/src/main/java/com/google/api/services/samples/adsense/cmdline/AdSenseSample.java
(left):

https://codereview.appspot.com/6297067/diff/1/adsense-cmdline-sample/src/main...
adsense-cmdline-sample/src/main/java/com/google/api/services/samples/adsense/cmdline/AdSenseSample.java:78:
.build();
(Mi)

[plumber1.MA, 2013-06-23 00:12:36]

Some one has hacked my phone restore asap

https://codereview.appspot.com/6297067/diff/1/adsense-cmdline-sample/src/main...
File
adsense-cmdline-sample/src/main/java/com/google/api/services/samples/adsense/cmdline/AdSenseSample.java
(left):

https://codereview.appspot.com/6297067/diff/1/adsense-cmdline-sample/src/main...
adsense-cmdline-sample/src/main/java/com/google/api/services/samples/adsense/cmdline/AdSenseSample.java:78:
.build();
(Mi)

[plumber1.MA, 2013-06-23 00:12:37]

Some one has hacked my phone restore asap

[plumber1.MA, 2013-06-23 00:12:37]

Some one has hacked my phone restore asap

[plumber1.MA, 2013-06-23 04:02:59]

https://codereview.appspot.com/6297067/diff/1/adsense-cmdline-sample/src/main...
File
adsense-cmdline-sample/src/main/java/com/google/api/services/samples/adsense/cmdline/AdSenseSample.java
(left):

https://codereview.appspot.com/6297067/diff/1/adsense-cmdline-sample/src/main...
adsense-cmdline-sample/src/main/java/com/google/api/services/samples/adsense/cmdline/AdSenseSample.java:127:
}
Don't),*

https://codereview.appspot.com/6297067/diff/1/adsense-cmdline-sample/src/main...
adsense-cmdline-sample/src/main/java/com/google/api/services/samples/adsense/cmdline/AdSenseSample.java:133:
}
(Stop).)';