From 93ebc1770bc84c1792e2a4b1b24af47ba9d0ff1e Mon Sep 17 00:00:00 2001
From: Stanley Feng <stfeng@google.com>
Date: Thu, 19 Nov 2015 15:35:30 -0800
Subject: [PATCH] [googledatastore] Incorporate CR comments from @kruthar

1. Move documentation from the properties file to the README.md
2. Addressed a couple of comments in the code.
3. Added the checkstyle plugin and addressed all the violation (except
the TODO comment violation. I'd like to keep a TODO item in the code).

[googledatastore] Output debug logs in one line instead of multiple lines.
---
 googledatastore/README.md                     | 55 ++++++++++++++++++-
 .../conf/googledatastore.properties           | 44 ++-------------
 googledatastore/pom.xml                       | 24 ++++++++
 .../yahoo/ycsb/db/GoogleDatastoreClient.java  | 33 ++++++-----
 .../java/com/yahoo/ycsb/db/package-info.java  | 22 ++++++++
 5 files changed, 122 insertions(+), 56 deletions(-)
 create mode 100644 googledatastore/src/main/java/com/yahoo/ycsb/db/package-info.java

diff --git a/googledatastore/README.md b/googledatastore/README.md
index 15ca6e78..0c52f576 100644
--- a/googledatastore/README.md
+++ b/googledatastore/README.md
@@ -25,7 +25,8 @@ https://cloud.google.com/datastore/docs/concepts/overview?hl=en
     YCSB_HOME - YCSB home directory
     DATASTORE_HOME - Google Cloud Datastore YCSB client package files
 
-Please refer to https://github.com/brianfrankcooper/YCSB/wiki/Using-the-Database-Libraries for more information on setup.
+Please refer to https://github.com/brianfrankcooper/YCSB/wiki/Using-the-Database-Libraries
+for more information on setup.
 
 # Benchmark
 
@@ -36,5 +37,55 @@ Please refer to https://github.com/brianfrankcooper/YCSB/wiki/Using-the-Database
 
     $DATASTORE_HOME/conf/googledatastore.properties
 
-# FAQs
+# Details
+
+A. Configuration and setup:
+
+See this link for instructions about setting up Google Cloud Datastore and
+authentication:
+
+https://cloud.google.com/datastore/docs/getstarted/start_java/
+
+After you setup your environment, you will have 3 pieces of information ready:
+- datasetId,
+- service account email, and
+- a private key file in P12 format.
+
+These will be configured via corresponding properties in the googledatastore.properties file.
+
+B. EntityGroupingMode
+
+In Google Datastore, Entity Group is the unit in which the user can
+perform strongly consistent query on multiple items; Meanwhile, Entity group
+also has certain limitations in performance, especially with write QPS.
+
+We support two modes here:
+
+1. [default] One entity per group (ONE_ENTITY_PER_GROUP)
+
+In this mode, every entity is a "root" entity and sits in one group,
+and every entity group has only one entity. Write QPS is high in this
+mode (and there is no documented limitation on this). But query across
+multiple entities are eventually consistent.
+
+When this mode is set, every entity is created with no ancestor key (meaning
+the entity itself is the "root" entity).
+
+2. Multiple entities per group (MULTI_ENTITY_PER_GROUP)
+
+In this mode, all entities in one benchmark run are placed under one
+ancestor (root) node therefore inside one entity group. Query/scan
+performed on these entities will be strongly consistent but write QPS
+will be subject to documented limitation (current is at 1 QPS).
+
+Because of the write QPS limit, it's highly recommended that you rate
+limit your benchmark's test rate to avoid excessive errors.
+
+The goal of this MULTI_ENTITY_PER_GROUP mode is to allow user to
+benchmark and understand performance characteristics of a single entity
+group of the Google Datastore.
+
+While in this mode, one can optionally specify a root key name. If not
+specified, a default name will be used.
+
 
diff --git a/googledatastore/conf/googledatastore.properties b/googledatastore/conf/googledatastore.properties
index 40fa93ce..0f1189c7 100644
--- a/googledatastore/conf/googledatastore.properties
+++ b/googledatastore/conf/googledatastore.properties
@@ -17,13 +17,8 @@
 # Sample property file for Google Cloud Datastore DB client 
 
 ## Mandatory parameters
-# See this link for instructions about setting up Google Cloud Datastore and
-# authentication:
-# https://cloud.google.com/datastore/docs/getstarted/start_java/
 #
-# After you setup your environment, you will have 3 pieces of information ready:
-# datasetId, service account email, and a private key file. These must be
-# configured via the properties below
+# Your credentials to Google datastore. See README.md for details.
 #
 # googledatastore.datasetId=<string id of your dataset>
 # googledatastore.privateKeyFile=<full path to your private key file>
@@ -44,39 +39,8 @@ readallfields = true
 #
 # googledatastore.readConsistency=EVENTUAL
 
-# Decides how we group entities into entity groups. In Google Datastore,
-# Entity Group is the unit in which the user can perform strongly
-# consistent query on multiple items; Meanwhile, Entity group also has
-# certain limitations in performance, especially with write QPS.
-#
-# We support two modes here:
-#
-# 1. [default] One entity per group (ONE_ENTITY_PER_GROUP)
-#
-# In this mode, every entity is a "root" entity and sits in one group,
-# and every entity group has only one entity. Write QPS is high in this
-# mode (and there is no documented limitation on this). But query across
-# multiple entities are eventually consistent.
-#
-# When this mode is set, every entity is created with no ancestor key (meaning 
-# the entity itself is the "root" entity).
-#
-# 2. Multiple entities per group (MULTI_ENTITY_PER_GROUP)
-#
-# In this mode, all entities in one benchmark run are placed under one
-# ancestor (root) node therefore inside one entity group. Query/scan
-# performed on these entities will be strongly consistent but write QPS
-# will be subject to documented limitation (current is at 1 QPS).
-#
-# Because of the write QPS limit, it's highly recommended that you rate
-# limit your benchmark's test rate to avoid excessive errors.
-#
-# The goal of this MULTI_ENTITY_PER_GROUP mode is to allow user to
-# benchmark and understand performance characteristics of a single entity
-# group of the Google Datastore.
-#
-# While in this mode, one can optionally specify a root key name. If not
-# specified, a default name will be used.
+# Decides how we group entities into entity groups. 
+# (See the details section in README.md for documentation)
 #
 # googledatastore.entityGroupingMode=ONE_ENTITY_PER_GROUP
 
@@ -89,4 +53,4 @@ readallfields = true
 # requestdistribution = uniform
 
 # Enable/disable debug message, default is false.
-# googledatastore.debug = false
+# googledatastore.debug = false
\ No newline at end of file
diff --git a/googledatastore/pom.xml b/googledatastore/pom.xml
index 52242bb8..19abce65 100644
--- a/googledatastore/pom.xml
+++ b/googledatastore/pom.xml
@@ -46,4 +46,28 @@ LICENSE file.
       <scope>provided</scope>
     </dependency>
   </dependencies>
+  <build>
+    <plugins>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-checkstyle-plugin</artifactId>
+        <version>2.15</version>
+        <configuration>
+          <consoleOutput>true</consoleOutput>
+          <configLocation>../checkstyle.xml</configLocation>
+          <failOnViolation>true</failOnViolation>
+          <failsOnError>false</failsOnError>
+        </configuration>
+        <executions>
+          <execution>
+            <id>validate</id>
+            <phase>validate</phase>
+            <goals>
+              <goal>checkstyle</goal>
+            </goals>
+          </execution>
+        </executions>
+      </plugin>
+    </plugins>
+  </build>
 </project>
diff --git a/googledatastore/src/main/java/com/yahoo/ycsb/db/GoogleDatastoreClient.java b/googledatastore/src/main/java/com/yahoo/ycsb/db/GoogleDatastoreClient.java
index d28d12bc..f2f64410 100644
--- a/googledatastore/src/main/java/com/yahoo/ycsb/db/GoogleDatastoreClient.java
+++ b/googledatastore/src/main/java/com/yahoo/ycsb/db/GoogleDatastoreClient.java
@@ -20,7 +20,8 @@ package com.yahoo.ycsb.db;
 import com.google.api.client.auth.oauth2.Credential;
 import com.google.api.services.datastore.DatastoreV1.*;
 import com.google.api.services.datastore.DatastoreV1.CommitRequest.Mode;
-import com.google.api.services.datastore.DatastoreV1.ReadOptions.ReadConsistency;
+import com.google.api.services.datastore.DatastoreV1.ReadOptions
+  .ReadConsistency;
 import com.google.api.services.datastore.client.Datastore;
 import com.google.api.services.datastore.client.DatastoreException;
 import com.google.api.services.datastore.client.DatastoreFactory;
@@ -93,7 +94,7 @@ public class GoogleDatastoreClient extends DB {
   public void init() throws DBException {
     String debug = getProperties().getProperty("googledatastore.debug", null);
     if (null != debug && "true".equalsIgnoreCase(debug)) {
-        logger.setLevel(Level.DEBUG);
+      logger.setLevel(Level.DEBUG);
     }
 
     // We need the following 3 essential properties to initialize datastore:
@@ -115,11 +116,11 @@ public class GoogleDatastoreClient extends DB {
           "Required property \"privateKeyFile\" missing.");
     }
 
-    String serviceAccount = getProperties().getProperty(
-        "googledatastore.serviceAccount", null);
-    if (serviceAccount == null) {
+    String serviceAccountEmail = getProperties().getProperty(
+        "googledatastore.serviceAccountEmail", null);
+    if (serviceAccountEmail == null) {
       throw new DBException(
-          "Required property \"serviceAccount\" missing.");
+          "Required property \"serviceAccountEmail\" missing.");
     }
 
     // Below are properties related to benchmarking.
@@ -161,10 +162,10 @@ public class GoogleDatastoreClient extends DB {
       // obtained from the configure.
       DatastoreOptions.Builder options = new DatastoreOptions.Builder();
       Credential credential = DatastoreHelper.getServiceAccountCredential(
-          serviceAccount, privateKeyFile);
+          serviceAccountEmail, privateKeyFile);
       logger.info("Using JWT Service Account credential.");
-      logger.info("DatasetID: " + datasetId + "\nService Account Email: " +
-          serviceAccount + "\nPrivate Key File Path: " + privateKeyFile);
+      logger.info("DatasetID: " + datasetId + ", Service Account Email: " +
+          serviceAccountEmail + ", Private Key File Path: " + privateKeyFile);
 
       datastore = DatastoreFactory.get().create(
           options.credential(credential).dataset(datasetId).build());
@@ -178,7 +179,7 @@ public class GoogleDatastoreClient extends DB {
             exception.getMessage());
     }
 
-    logger.info("Datastore client instance created:\n" +
+    logger.info("Datastore client instance created: " +
         datastore.toString());
   }
 
@@ -192,7 +193,7 @@ public class GoogleDatastoreClient extends DB {
     // Note above, datastore lookupRequest always reads the entire entity, it
     // does not support reading a subset of "fields" (properties) of an entity.
 
-    logger.debug("Built lookup request as:\n" + lookupRequest.toString());
+    logger.debug("Built lookup request as: " + lookupRequest.toString());
 
     LookupResponse response = null;
     try {
@@ -212,6 +213,10 @@ public class GoogleDatastoreClient extends DB {
 
     if (response.getFoundCount() == 0) {
       return new Status("ERROR-404", "Not Found, key is: " + key);
+    } else if (response.getFoundCount() > 1) {
+      // We only asked to lookup for one key, shouldn't have got more than one
+      // entity back. Unexpected State.
+      return Status.UNEXPECTED_STATE;
     }
 
     Entity entity = response.getFound(0).getEntity();
@@ -234,8 +239,8 @@ public class GoogleDatastoreClient extends DB {
   @Override
   public Status scan(String table, String startkey, int recordcount,
       Set<String> fields, Vector<HashMap<String, ByteIterator>> result) {
-      // TODO: Implement Scan as query on primary key.
-      return Status.NOT_IMPLEMENTED;
+    // TODO: Implement Scan as query on primary key.
+    return Status.NOT_IMPLEMENTED;
   }
 
   @Override
@@ -330,5 +335,5 @@ public class GoogleDatastoreClient extends DB {
     }
 
     return Status.OK;
- }
+  }
 }
diff --git a/googledatastore/src/main/java/com/yahoo/ycsb/db/package-info.java b/googledatastore/src/main/java/com/yahoo/ycsb/db/package-info.java
new file mode 100644
index 00000000..c0560d64
--- /dev/null
+++ b/googledatastore/src/main/java/com/yahoo/ycsb/db/package-info.java
@@ -0,0 +1,22 @@
+/**
+ * Copyright (c) 2015 Google Inc. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you
+ * may not use this file except in compliance with the License. You
+ * may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+ * implied. See the License for the specific language governing
+ * permissions and limitations under the License. See accompanying
+ * LICENSE file.
+ */
+
+/**
+ * YCSB binding for
+<a href="https://cloud.google.com/datastore/">Google Cloud Datastore</a>.
+ */
+package com.yahoo.ycsb.db;
-- 
GitLab