protobuf 3 compat issues

[pcarrier]

• datastore-v1-protos ships com.google.protobuf.Timestamp which implements com.google.protobuf.GeneratedMessage
• protobuf-java 3.0.2 ships com.google.protobuf.Timestamp which implements com.google.protobuf.GeneratedMessageV3
• As a result, with protobuf-java picked by the classloader, google-cloud-datastore classes (eg com.google.datastore.v1.Value.toBuilder) trigger java.lang.VerifyError (Type 'com/google/protobuf/Timestamp' (current frame, stack[1]) is not assignable to 'com/google/protobuf/GeneratedMessage')
• Excluding datastore-v1-protos from the classpath and generating the classes from proto ourselves then breaks google-cloud-datastore, eg because of its com/google/cloud/datastore/PathElement.toPb()Lcom/google/protobuf/GeneratedMessage;.

I don't even know where to report this, maybe the protobuf team, but I figured the Google Cloud Platform team was a safer bet given it impacts their customers.

[pcarrier]

FWIW, worked around by:

• Excluding group: 'com.google.cloud.datastore', module: 'datastore-v1-protos'
• Instead, generating Java classes with protoc 3.0.2 for:
  • google/datastore/v1/datastore.proto
  • google/datastore/v1/entity.proto
  • google/datastore/v1/query.proto
  • google/type/latlng.proto
• Shamelessly forking google-cloud-datastore with:

commit 4370f038b70e329d716684d846099effb9ef2cc7
Author: Pierre Carrier <[email protected]>
Date:   2016-09-10 00:31:51 -0700

    The Fix

diff --git forks/google-cloud-datastore/src/main/java/com/google/cloud/datastore/Query.java forks/google-cloud-datastore/src/main/java/com/google/cloud/datastore/Query.java
index eb33c02..7eb7b17 100644
--- forks/google-cloud-datastore/src/main/java/com/google/cloud/datastore/Query.java
+++ forks/google-cloud-datastore/src/main/java/com/google/cloud/datastore/Query.java
@@ -22,6 +22,7 @@ import com.google.common.base.MoreObjects;
 import com.google.common.base.MoreObjects.ToStringHelper;
 import com.google.common.collect.Maps;
 import com.google.protobuf.GeneratedMessage;
+import com.google.protobuf.GeneratedMessageV3;
 import com.google.protobuf.InvalidProtocolBufferException;

 import java.util.Map;
@@ -38,7 +39,7 @@ import java.util.Map;
  * @param <V> the type of the values returned by this query.
  * @see <a href="https://cloud.google.com/datastore/docs/concepts/queries">Datastore Queries</a>
  */
-public abstract class Query<V> extends Serializable<GeneratedMessage> {
+public abstract class Query<V> extends Serializable<GeneratedMessageV3> {

   private static final long serialVersionUID = -2748141759901313101L;

diff --git forks/google-cloud-datastore/src/main/java/com/google/cloud/datastore/Serializable.java forks/google-cloud-datastore/src/main/java/com/google/cloud/datastore/Serializable.java
index 2e30593..3f9205c 100644
--- forks/google-cloud-datastore/src/main/java/com/google/cloud/datastore/Serializable.java
+++ forks/google-cloud-datastore/src/main/java/com/google/cloud/datastore/Serializable.java
@@ -16,7 +16,7 @@

 package com.google.cloud.datastore;

-import com.google.protobuf.GeneratedMessage;
+import com.google.protobuf.GeneratedMessageV3;
 import com.google.protobuf.InvalidProtocolBufferException;

 import java.io.IOException;
@@ -25,7 +25,7 @@ import java.io.ObjectOutputStream;
 import java.io.ObjectStreamException;
 import java.io.StreamCorruptedException;

-abstract class Serializable<M extends GeneratedMessage> implements java.io.Serializable {
+abstract class Serializable<M extends GeneratedMessageV3> implements java.io.Serializable {

   private static final long serialVersionUID = -5565522710061949199L;

[mziccard]

Hi @pcarrier thanks for the report, the digging, and finding a workaround. protobuf-java was recently released and most of our dependencies still use 3.0.0 betas (which did not include the GeneratedMessageV3 class). I believe you could stumble upon similar issues with other of our modules.

Sorting this out won't be easy but should at least be possible. Something that we planned to do (haven't had time yet) is to get rid of the Serializable<M extends GeneratedMessage> class, which should at least save you from forking our repos.

Another issue I see (I might be wrong though), is that datastore-v1-protos define its own com.google.protobuf.* classes, instead of depending on protobuf-java. @eddavisson is there a reason for this? Now that protobuf-java 3.0.0 is out we should depend on it. And if we cannot depend on it for whatever reason, those classes should be shaded. Otherwise we will face the same issues @pcarrier is facing as soon as we switch to protobuf-java 3.0.0 for all Veneer Toolkit services.

@garrettjonesgoogle I believe the introduction of the GeneratedMessageV3 class could also give you an headache as well. Any plans for switching everything to protobuf v3.0.0? Its important that we coordinate our efforts so that both datastore-v1-protos and VeneerToolkit update to 3.0.0 happen at the same time.

[garrettjonesgoogle]

We were going to do the switch within a week or two, but hadn't chosen a specific time for it. Let me know if there is a time that works better than others.

[eddavisson]

@mziccard I don't remember why we ended up including those com.google.protobuf.* classes, but I agree that it looks like we should be able to pick them up from protobuf-java instead.

Are any of your other dependencies (e.g. BigQuery) packaging their own generate protocol buffer classes, or do they all use the google-api-services-.* packages?

[mziccard]

Are any of your other dependencies (e.g. BigQuery) packaging their own generate protocol buffer classes, or do they all use the google-api-services-.* packages?

All services on top of gRPC are using their own google-api-services-.* package. I believe VeneerToolkit guys are taking care of releasing them.

[eddavisson]

Got it. Any plans to switch the Datastore client to gRPC?

[mziccard]

No plans yet. Do you believe you can change to use protobuf-java any time soon? You should be able to directly jump diretcty to 3.0.0.

[eddavisson]

I can have something by the end of the week.

[eddavisson]

New artifacts that use protobuf-java 3.0.0 (and don't include the com.google.protobuf.* classes):

<dependency>
  <groupId>com.google.cloud.datastore</groupId>
  <artifactId>datastore-v1-protos</artifactId>
  <version>1.2.0</version>
</dependency>
<dependency>
  <groupId>com.google.cloud.datastore</groupId>
  <artifactId>datastore-v1-proto-client</artifactId>
  <version>1.2.0</version>
</dependency>

[mziccard]

Thank you @eddavisson, this was fast:)

I'll go ahead and use the new artifacts as soon as also the VeneerToolkit layer has updated to protobuf-java 3.0.0

[mziccard]

@pcarrier this issue should be now fixed in google-cloud 0.4.0. Care to check it out?

[mziccard]

I am closing this, please feel free to reopen if you are still experiencing the issue

[pcarrier]

Checked yesterday, upgrade went smoothly AFAICT. Thanks!