From 4339b1c57fc1a2cf1df5467367cc67fea4f8cbe0 Mon Sep 17 00:00:00 2001 From: Michael Darakananda Date: Fri, 2 Feb 2018 15:40:41 +1100 Subject: [PATCH 1/4] RFC: new approach for snippet injection The current snippet injector does not work properly with google-java-format, because GJF formats short javadoc comments on one line, eg "/** comment */". However, the injector script looks for "/**" on a line by itself. The script will also not work if/when we move to Java 8, due to lack of parser support. This PR takes a different approach of not caring about Java syntax and copy-paste everything in the SNIPPET block. While less powerful, it is more robust. As written, the script is also easier to use. There's no need to tell it what file contains snippets and where to copy the snippets to. The script recursively scan given directories. Updates #2413. --- .../cloud/pubsub/v1/MessageReceiver.java | 38 ++-- .../snippets/MessageReceiverSnippets.java | 36 ++-- utilities/snippets.go | 173 ++++++++++++++++++ 3 files changed, 212 insertions(+), 35 deletions(-) create mode 100644 utilities/snippets.go diff --git a/google-cloud-clients/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageReceiver.java b/google-cloud-clients/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageReceiver.java index c247260d3831..d1428a6d5cc2 100644 --- a/google-cloud-clients/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageReceiver.java +++ b/google-cloud-clients/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageReceiver.java @@ -22,27 +22,31 @@ public interface MessageReceiver { /** * Called when a message is received by the subscriber. The implementation must arrange for {@link - * AckReplyConsumer#ack()} or {@link - * AckReplyConsumer#nack()} to be called after processing the {@code message}. + * AckReplyConsumer#ack()} or {@link AckReplyConsumer#nack()} to be called after processing the + * {@code message}. + * * - *

This {@code MessageReceiver} passes all messages to a {@code BlockingQueue}. - * This method can be called concurrently from multiple threads, - * so it is important that the queue be thread-safe. + *

{@code
+   * // This {@code MessageReceiver} passes all messages to a {@link BlockingQueue}. This method can
+   * // be called concurrently from multiple threads, so it is important that the queue be
+   * // thread-safe.
+   * //
+   * // This example is for illustration. Implementations may directly process messages instead of
+   * // sending them to queues.
+   * MessageReceiver receiver =
+   *     new MessageReceiver() {
+   *       public void receiveMessage(final PubsubMessage message, final AckReplyConsumer consumer) {
+   *         if (blockingQueue.offer(message)) {
+   *           consumer.ack();
+   *         } else {
+   *           consumer.nack();
+   *         }
+   *       }
+   *     };
    *
-   * This example is for illustration. Implementations may directly process messages
-   * instead of sending them to queues.
-   * 
 {@code
-   * MessageReceiver receiver = new MessageReceiver() {
-   *   public void receiveMessage(final PubsubMessage message, final AckReplyConsumer consumer) {
-   *     if (blockingQueue.offer(message)) {
-   *       consumer.ack();
-   *     } else {
-   *       consumer.nack();
-   *     }
-   *   }
-   * };
    * }
* + * */ void receiveMessage(final PubsubMessage message, final AckReplyConsumer consumer); } diff --git a/google-cloud-examples/src/main/java/com/google/cloud/examples/pubsub/snippets/MessageReceiverSnippets.java b/google-cloud-examples/src/main/java/com/google/cloud/examples/pubsub/snippets/MessageReceiverSnippets.java index 48aa27bdbf57..f082afa40f0a 100644 --- a/google-cloud-examples/src/main/java/com/google/cloud/examples/pubsub/snippets/MessageReceiverSnippets.java +++ b/google-cloud-examples/src/main/java/com/google/cloud/examples/pubsub/snippets/MessageReceiverSnippets.java @@ -36,25 +36,25 @@ public MessageReceiverSnippets(BlockingQueue blockingQueue) { this.blockingQueue = blockingQueue; } - /** - * This {@code MessageReceiver} passes all messages to a {@link BlockingQueue}. - * This method can be called concurrently from multiple threads, - * so it is important that the queue be thread-safe. - * - * This example is for illustration. Implementations may directly process messages - * instead of sending them to queues. - */ - // [TARGET receiveMessage(PubsubMessage, AckReplyConsumer)] public MessageReceiver messageReceiver() { - MessageReceiver receiver = new MessageReceiver() { - public void receiveMessage(final PubsubMessage message, final AckReplyConsumer consumer) { - if (blockingQueue.offer(message)) { - consumer.ack(); - } else { - consumer.nack(); - } - } - }; + // SNIPPET receiveMessage + // This {@code MessageReceiver} passes all messages to a {@link BlockingQueue}. This method can + // be called concurrently from multiple threads, so it is important that the queue be + // thread-safe. + // + // This example is for illustration. Implementations may directly process messages instead of + // sending them to queues. + MessageReceiver receiver = + new MessageReceiver() { + public void receiveMessage(final PubsubMessage message, final AckReplyConsumer consumer) { + if (blockingQueue.offer(message)) { + consumer.ack(); + } else { + consumer.nack(); + } + } + }; + // SNIPPET receiveMessage return receiver; } } diff --git a/utilities/snippets.go b/utilities/snippets.go new file mode 100644 index 000000000000..327705daec94 --- /dev/null +++ b/utilities/snippets.go @@ -0,0 +1,173 @@ +package main + +import ( + "bytes" + "flag" + "fmt" + "io/ioutil" + "log" + "os" + "path/filepath" + "runtime/pprof" + "strings" +) + +func init() { + log.SetFlags(0) + log.SetPrefix("snippet: ") +} + +func main() { + cpuprof := flag.String("cpuprofile", "", "write CPU profile to this file") + flag.Parse() + + if cp := *cpuprof; cp != "" { + f, err := os.Create(cp) + if err != nil { + log.Fatal(err) + } + defer f.Close() + + pprof.StartCPUProfile(f) + defer pprof.StopCPUProfile() + } + + files := map[string]string{} + walkFn := func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if !info.Mode().IsRegular() || filepath.Ext(path) != ".java" { + return nil + } + b, err := ioutil.ReadFile(path) + if err != nil { + return err + } + files[path] = string(b) + return nil + } + for _, dir := range flag.Args() { + if err := filepath.Walk(dir, walkFn); err != nil { + log.Fatal(err) + } + } + + snip := map[string]string{} + for file, txt := range files { + if err := getSnip(file, txt, snip); err != nil { + log.Fatal(err) + } + } + + rd := rewriteData{ + rewrite: map[string]string{}, + used: map[string]bool{}, + } + for file, txt := range files { + if err := writeSnip(file, txt, snip, rd); err != nil { + log.Fatal(err) + } + } + + for file, txt := range rd.rewrite { + if err := ioutil.WriteFile(file, []byte(txt), 0644); err != nil { + log.Fatal(err) + } + } + + for key := range snip { + if !rd.used[key] { + log.Printf("unused snippet: %q", key) + } + } +} + +func getSnip(file, txt string, snip map[string]string) error { + const snipPrefix = "// SNIPPET " + + ftxt := txt + for { + if p := strings.Index(txt, snipPrefix); p >= 0 { + txt = txt[p:] + } else { + return nil + } + + var key string + if p := strings.IndexByte(txt, '\n'); p >= 0 { + key = strings.TrimLeft(txt[:p], "\r") + txt = txt[p:] + } else { + key = txt + txt = "" + } + + if p := strings.Index(txt, key); p >= 0 { + // "// SNIPPET foo" -> "" + key = fmt.Sprintf("", key[3:]) + + if _, exist := snip[key]; exist { + return fmt.Errorf("%s:%d snippet %q has already been defined", file, lineNum(ftxt, txt), key) + } + + snip[key] = strings.Trim(txt[:p], "\n\r") + txt = txt[p+len(snipPrefix):] + } + } +} + +type rewriteData struct { + rewrite map[string]string + used map[string]bool +} + +func writeSnip(file, txt string, snip map[string]string, rd rewriteData) error { + const snipPrefix = ""); p >= 0 { + key = txt[:p+3] + txt = txt[p+3:] + } + + rep, ok := snip[key] + if ok { + rd.used[key] = true + } else { + return fmt.Errorf("%s:%d snippet %q undefined", file, lineNum(ftxt, txt), key) + } + + if p := strings.Index(txt, key); p >= 0 { + buf.WriteString(key) + buf.WriteString("\n*
{@code\n *")
+			buf.WriteString(strings.Replace(rep, "\n", "\n*", -1))
+			buf.WriteString("\n* }
\n") + buf.WriteString(key) + txt = txt[p+len(key):] + } else { + return fmt.Errorf("%s:%d snippet %q not closed", file, lineNum(ftxt, txt), key) + } + } +} + +// Give strings s and suf where suf is a suffix of s, lineNum reports +// the line number on which suf starts. +func lineNum(s, suf string) int { + pre := s[:len(s)-len(suf)] + return strings.Count(pre, "\n") + 1 +} From 725eefcc667553303945097311c9396cf41dd8e9 Mon Sep 17 00:00:00 2001 From: Michael Darakananda Date: Mon, 12 Feb 2018 17:31:38 +1100 Subject: [PATCH 2/4] license --- utilities/snippets.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/utilities/snippets.go b/utilities/snippets.go index 327705daec94..4c6da8c2b085 100644 --- a/utilities/snippets.go +++ b/utilities/snippets.go @@ -1,3 +1,17 @@ +// Copyright 2018 Google LLC +// +// 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. + package main import ( @@ -96,7 +110,7 @@ func getSnip(file, txt string, snip map[string]string) error { var key string if p := strings.IndexByte(txt, '\n'); p >= 0 { - key = strings.TrimLeft(txt[:p], "\r") + key = txt[:p] txt = txt[p:] } else { key = txt @@ -105,7 +119,7 @@ func getSnip(file, txt string, snip map[string]string) error { if p := strings.Index(txt, key); p >= 0 { // "// SNIPPET foo" -> "" - key = fmt.Sprintf("", key[3:]) + key = fmt.Sprintf("", strings.TrimSpace(key[3:])) if _, exist := snip[key]; exist { return fmt.Errorf("%s:%d snippet %q has already been defined", file, lineNum(ftxt, txt), key) @@ -113,6 +127,8 @@ func getSnip(file, txt string, snip map[string]string) error { snip[key] = strings.Trim(txt[:p], "\n\r") txt = txt[p+len(snipPrefix):] + } else { + return fmt.Errorf("%s:%d snippet %q not closed", file, lineNum(ftxt, txt), key) } } } From 84ec1a9b22879060df8f9cd61efcc9411bc1ab74 Mon Sep 17 00:00:00 2001 From: Tim Swast Date: Tue, 3 Jul 2018 17:04:39 -0700 Subject: [PATCH 3/4] Add test case for getSnip --- utilities/snippets_test.go | 93 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 utilities/snippets_test.go diff --git a/utilities/snippets_test.go b/utilities/snippets_test.go new file mode 100644 index 000000000000..54e07ed3bd40 --- /dev/null +++ b/utilities/snippets_test.go @@ -0,0 +1,93 @@ +// Copyright 2018 Google LLC +// +// 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. +package main + +import ( + "reflect" + "testing" +) + +var sniptests = []struct { + txt string + out map[string]string +}{ + { + `package com.google.cloud.examples.pubsub.snippets; + +import com.google.cloud.pubsub.v1.AckReplyConsumer; +import com.google.cloud.pubsub.v1.MessageReceiver; +import com.google.pubsub.v1.PubsubMessage; +import java.util.concurrent.BlockingQueue; + +/** This class contains snippets for the {@link MessageReceiver} interface. */ + +public class MessageReceiverSnippets { + private final BlockingQueue blockingQueue; + + public MessageReceiverSnippets(BlockingQueue blockingQueue) { + this.blockingQueue = blockingQueue; + } + + public MessageReceiver messageReceiver() { + // SNIPPET receiveMessage + MessageReceiver receiver = + new MessageReceiver() { + public void receiveMessage(final PubsubMessage message, final AckReplyConsumer consumer) { + if (blockingQueue.offer(message)) { + consumer.ack(); + } else { + consumer.nack(); + } + } + }; + // SNIPPET receiveMessage + return receiver; + } + + // SNIPPET secondSnippet + // some example code. + // SNIPPET secondSnippet +} +`, + map[string]string{ + "": ` MessageReceiver receiver = + new MessageReceiver() { + public void receiveMessage(final PubsubMessage message, final AckReplyConsumer consumer) { + if (blockingQueue.offer(message)) { + consumer.ack(); + } else { + consumer.nack(); + } + } + }; + `, + "": "\t// some example code.\n\t", + }, + }, +} + +func TestGetSnip(t *testing.T) { + for _, tt := range sniptests { + t.Run(tt.txt, func(t *testing.T) { + + got := map[string]string{} + if err := getSnip("testfile.java", tt.txt, got); err != nil { + t.Errorf("error getting snips: %s", err) + } + if !reflect.DeepEqual(tt.out, got) { + t.Errorf("expected %v, got %v", tt.out, got) + } + }) + } +} From 5382673475e634731720c7ed1eb04ccacb22068d Mon Sep 17 00:00:00 2001 From: Tim Swast Date: Tue, 3 Jul 2018 17:43:06 -0700 Subject: [PATCH 4/4] Add support for cloud region tags to snippet.go --- .../com/google/cloud/bigquery/BigQuery.java | 24 +++--- .../bigquery/snippets/BigQuerySnippets.java | 8 +- utilities/snippets.go | 51 +++++++++++-- utilities/snippets_test.go | 74 +++++++++++++++++++ 4 files changed, 139 insertions(+), 18 deletions(-) diff --git a/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQuery.java b/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQuery.java index ee370bea5a58..5c5dcd496d61 100644 --- a/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQuery.java +++ b/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQuery.java @@ -753,13 +753,19 @@ public int hashCode() { * Updates dataset information. * *

Example of updating a dataset by changing its description. - *

 {@code
-   * String datasetName = "my_dataset_name";
-   * String newDescription = "some_new_description";
-   * Dataset oldDataset = bigquery.getDataset(datasetName);
-   * DatasetInfo datasetInfo = oldDataset.toBuilder().setDescription(newDescription).build();
-   * Dataset newDataset = bigquery.update(datasetInfo);
-   * }
+ * + *
{@code
+   *    // String datasetName = "my_dataset_name";
+   *    // String tableName = "my_table_name";
+   *    // String newDescription = "new_description";
+   *
+   *    Table beforeTable = bigquery.getTable(datasetName, tableName);
+   *    TableInfo tableInfo = beforeTable.toBuilder()
+   *        .setDescription(newDescription)
+   *        .build();
+   *    Table afterTable = bigquery.update(tableInfo);
+   *     }
+ * * * @throws BigQueryException upon failure */ @@ -785,7 +791,7 @@ public int hashCode() { * String datasetName = "my_dataset_name"; * String tableName = "my_table_name"; * Table beforeTable = bigquery.getTable(datasetName, tableName); - * + * * // Set table to expire 5 days from now. * long expirationMillis = DateTime.now().plusDays(5).getMillis(); * TableInfo tableInfo = beforeTable.toBuilder() @@ -1104,7 +1110,7 @@ TableResult listTableData( * // BigQuery bigquery = BigQueryOptions.getDefaultInstance().getService(); * String query = "SELECT corpus FROM `bigquery-public-data.samples.shakespeare` GROUP BY corpus;"; * QueryJobConfiguration queryConfig = QueryJobConfiguration.newBuilder(query).build(); - * + * * // Print the results. * for (FieldValueList row : bigquery.query(queryConfig).iterateAll()) { * for (FieldValue val : row) { diff --git a/google-cloud-examples/src/main/java/com/google/cloud/examples/bigquery/snippets/BigQuerySnippets.java b/google-cloud-examples/src/main/java/com/google/cloud/examples/bigquery/snippets/BigQuerySnippets.java index b1edd909c041..5724d1c93c37 100644 --- a/google-cloud-examples/src/main/java/com/google/cloud/examples/bigquery/snippets/BigQuerySnippets.java +++ b/google-cloud-examples/src/main/java/com/google/cloud/examples/bigquery/snippets/BigQuerySnippets.java @@ -121,12 +121,12 @@ public Dataset updateDataset(String datasetName, String newDescription) { /** * Example of updating a table by changing its description. */ - // [TARGET update(TableInfo, TableOption...)] - // [VARIABLE "my_dataset_name"] - // [VARIABLE "my_table_name"] - // [VARIABLE "new_description"] public Table updateTableDescription(String datasetName, String tableName, String newDescription) { // [START bigquery_update_table_description] + // String datasetName = "my_dataset_name"; + // String tableName = "my_table_name"; + // String newDescription = "new_description"; + Table beforeTable = bigquery.getTable(datasetName, tableName); TableInfo tableInfo = beforeTable.toBuilder() .setDescription(newDescription) diff --git a/utilities/snippets.go b/utilities/snippets.go index 4c6da8c2b085..5a9c66204c37 100644 --- a/utilities/snippets.go +++ b/utilities/snippets.go @@ -72,6 +72,9 @@ func main() { if err := getSnip(file, txt, snip); err != nil { log.Fatal(err) } + if err := getCloud(file, txt, snip); err != nil { + log.Fatal(err) + } } rd := rewriteData{ @@ -97,6 +100,44 @@ func main() { } } +func getCloud(file, txt string, snip map[string]string) error { + const cloudPrefix = "// [START " + const cloudSuffix = "// [END %s]" + + ftxt := txt + for { + if p := strings.Index(txt, cloudPrefix); p >= 0 { + txt = txt[p:] + } else { + return nil + } + + var tag string + if p := strings.Index(txt, "]\n"); p >= 0 { + // "// [START foo]" -> "foo" + tag = txt[10:p] + txt = txt[p+1:] + } else { + tag = txt + txt = "" + } + + endTag := fmt.Sprintf(cloudSuffix, tag) + if p := strings.Index(txt, endTag); p >= 0 { + key := fmt.Sprintf("", tag) + snipTxt := strings.Trim(txt[:p], "\n\r") + if _, exist := snip[key]; exist { + snip[key] = strings.Join([]string{snip[key], snipTxt}, "") + } + + snip[key] = snipTxt + txt = txt[p+len(endTag):] + } else { + return fmt.Errorf("[START %s]:%d snippet %q not closed", file, lineNum(ftxt, txt), tag) + } + } +} + func getSnip(file, txt string, snip map[string]string) error { const snipPrefix = "// SNIPPET " @@ -165,18 +206,18 @@ func writeSnip(file, txt string, snip map[string]string, rd rewriteData) error { if ok { rd.used[key] = true } else { - return fmt.Errorf("%s:%d snippet %q undefined", file, lineNum(ftxt, txt), key) + return fmt.Errorf("%s:%d snippet target %q undefined", file, lineNum(ftxt, txt), key) } if p := strings.Index(txt, key); p >= 0 { buf.WriteString(key) - buf.WriteString("\n*
{@code\n *")
-			buf.WriteString(strings.Replace(rep, "\n", "\n*", -1))
-			buf.WriteString("\n* }
\n") + buf.WriteString("\n *
{@code\n   *")
+			buf.WriteString(strings.Replace(rep, "\n", "\n   *", -1))
+			buf.WriteString(" }
\n * ") buf.WriteString(key) txt = txt[p+len(key):] } else { - return fmt.Errorf("%s:%d snippet %q not closed", file, lineNum(ftxt, txt), key) + return fmt.Errorf("%s:%d snippet target %q not closed", file, lineNum(ftxt, txt), key) } } } diff --git a/utilities/snippets_test.go b/utilities/snippets_test.go index 54e07ed3bd40..0938a2bd4638 100644 --- a/utilities/snippets_test.go +++ b/utilities/snippets_test.go @@ -91,3 +91,77 @@ func TestGetSnip(t *testing.T) { }) } } + +var cloudtests = []struct { + txt string + out map[string]string +}{ + { + `package com.google.cloud.examples.pubsub.snippets; + +import com.google.cloud.pubsub.v1.AckReplyConsumer; +import com.google.cloud.pubsub.v1.MessageReceiver; +import com.google.pubsub.v1.PubsubMessage; +import java.util.concurrent.BlockingQueue; + +/** This class contains snippets for the {@link MessageReceiver} interface. */ + +public class MessageReceiverSnippets { + private final BlockingQueue blockingQueue; + + public MessageReceiverSnippets(BlockingQueue blockingQueue) { + this.blockingQueue = blockingQueue; + } + + public MessageReceiver messageReceiver() { + // [START pubsub_receive_message] + MessageReceiver receiver = + new MessageReceiver() { + public void receiveMessage(final PubsubMessage message, final AckReplyConsumer consumer) { + if (blockingQueue.offer(message)) { + consumer.ack(); + } else { + consumer.nack(); + } + } + }; + // [END pubsub_receive_message] + return receiver; + } + + // [START pubsub_second_snippet] + // some example code. + // [END pubsub_second_snippet] +} +`, + map[string]string{ + "": ` MessageReceiver receiver = + new MessageReceiver() { + public void receiveMessage(final PubsubMessage message, final AckReplyConsumer consumer) { + if (blockingQueue.offer(message)) { + consumer.ack(); + } else { + consumer.nack(); + } + } + }; + `, + "": "\t// some example code.\n\t", + }, + }, +} + +func TestGetCloud(t *testing.T) { + for _, tt := range cloudtests { + t.Run(tt.txt, func(t *testing.T) { + + got := map[string]string{} + if err := getCloud("testfile.java", tt.txt, got); err != nil { + t.Errorf("error getting cloud snips: %s", err) + } + if !reflect.DeepEqual(tt.out, got) { + t.Errorf("expected %v, got %v", tt.out, got) + } + }) + } +}