Repository: knative-sandbox/kn-plugin-func
Pull request #879 (2022-03-04 17:35:32)

Generate templates zip unconditionally
Changes:
* Generate templates zip unconditionally (i.e. no `make` prerequisites).
  Rationale:
  * The way we create prerequisites (the `find` command) we cannot detect deleted files.
  * The generation is fast (around 500ms) so we can afford that.
* Some tiny performance tweaks.
* Test embedded templates content on push to `main`.
   The test has to run prior to `make test` because the `test` recipe enforces generation of the templates zip.
   It would prevent test from being run against git committed template.

knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative-sandbox/kn-plugin-func/pull/879#" title="Author self-approved">matejvasek</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative-sandbox%2Fkn-plugin-func).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/knative-sandbox/kn-plugin-func/blob/main/OWNERS)~~ [matejvasek]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
codecov[bot]: # [Codecov](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/879?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) Report
> Merging [#879](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/879?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) (2a15ee7) into [main](https://codecov.io/gh/knative-sandbox/kn-plugin-func/commit/1e4d52be33f68df2137d63e57475994f1353428f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) (1e4d52b) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/879/graphs/tree.svg?width=650&height=150&src=pr&token=da1B9bMsQ9&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox)](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/879?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox)

```diff
@@           Coverage Diff           @@
##             main     #879   +/-   ##
=======================================
  Coverage   45.18%   45.18%           
=======================================
  Files          48       48           
  Lines        4818     4818           
=======================================
  Hits         2177     2177           
  Misses       2342     2342           
  Partials      299      299           
```


| [Impacted Files](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/879?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) | Coverage Δ | |
|---|---|---|
| [filesystem.go](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/879/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox#diff-ZmlsZXN5c3RlbS5nbw==) | `59.49% <ø> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/879?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/879?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox). Last update [1e4d52b...2a15ee7](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/879?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox).

matejvasek: @lance @lkingland @zroubalik PTAL

Patch:
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
index a413ef1a..390017b1 100644
--- a/.github/workflows/ci.yaml
+++ b/.github/workflows/ci.yaml
@@ -15,6 +15,8 @@ jobs:
       - uses: actions/setup-java@v1
         with:
           java-version: '11'
+      - name: Check embedded templates content
+        run: go test -run "^\QTestFileSystems\E$/^\Qembedded\E$"
       - name: Unit Test
         run:  make test
       - name: Template Unit Tests
diff --git a/Makefile b/Makefile
index 1748ed07..7fdf6b0a 100644
--- a/Makefile
+++ b/Makefile
@@ -60,7 +60,9 @@ check: bin/golangci-lint ## Check code quality (lint)
 bin/golangci-lint:
 	curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b ./bin v1.43.0
 
-zz_filesystem_generated.go: $(TEMPLATES)
+.PHONY: zz_filesystem_generated.go
+
+zz_filesystem_generated.go:
 	# Removing temporary template files
 	@rm -rf templates/node/cloudevents/node_modules
 	@rm -rf templates/node/http/node_modules
diff --git a/client.go b/client.go
index 9d4f0a83..dde8e477 100644
--- a/client.go
+++ b/client.go
@@ -846,7 +846,7 @@ func (c *Client) Remove(ctx context.Context, cfg Function, deleteAll bool) error
 // See NewInvokeMessage for its defaults.
 // Functions are invoked in a manner consistent with the settings defined in
 // their metadata.  For example HTTP vs CloudEvent
-func (c *Client) Invoke(ctx context.Context, root string, target string, m InvokeMessage) (err error) {
+func (c *Client) Invoke(ctx context.Context, root string, target string, m InvokeMessage) (s string, err error) {
 	go func() {
 		<-ctx.Done()
 		c.progressListener.Stopping()
diff --git a/cmd/invoke.go b/cmd/invoke.go
index 7fecc9d8..a1128985 100644
--- a/cmd/invoke.go
+++ b/cmd/invoke.go
@@ -160,12 +160,12 @@ func runInvoke(cmd *cobra.Command, args []string, newClient ClientFactory) (err
 	}
 
 	// Invoke
-	err = client.Invoke(cmd.Context(), cfg.Path, cfg.Target, m)
+	s, err := client.Invoke(cmd.Context(), cfg.Path, cfg.Target, m)
 	if err != nil {
 		return err
 	}
 
-	fmt.Fprintf(cmd.OutOrStderr(), "Invoked %v\n", cfg.Target)
+	fmt.Fprintln(cmd.OutOrStderr(), s)
 	return
 }
 
diff --git a/generate/templates/main.go b/generate/templates/main.go
index cd89f285..e990b239 100644
--- a/generate/templates/main.go
+++ b/generate/templates/main.go
@@ -3,8 +3,6 @@ package main
 import (
 	"archive/zip"
 	"bufio"
-	"bytes"
-	"errors"
 	"fmt"
 	"io"
 	"io/fs"
@@ -16,9 +14,23 @@ import (
 // This program generates zz_filesystem_generated.go file containing byte array variable named templatesZip.
 // The variable contains zip of "./templates" directory.
 func main() {
-	var zipBuff bytes.Buffer
-	zipWriter := zip.NewWriter(&zipBuff)
-	err := filepath.Walk("templates", func(path string, info fs.FileInfo, err error) error {
+	f, err := os.OpenFile("zz_filesystem_generated.go", os.O_TRUNC|os.O_CREATE|os.O_WRONLY, 0644)
+	if err != nil {
+		log.Fatal(err)
+	}
+	defer f.Close()
+
+	srcOut := bufio.NewWriter(f)
+	defer srcOut.Flush()
+
+	_, err = fmt.Fprintf(srcOut, "// Code generated by go generate; DO NOT EDIT.\npackage function\n\nvar templatesZip = []byte{\n")
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	zipWriter := zip.NewWriter(newGoByteArrayWriter(srcOut))
+	buff := make([]byte, 4*1024)
+	err = filepath.Walk("templates", func(path string, info fs.FileInfo, err error) error {
 		if err != nil {
 			return err
 		}
@@ -48,12 +60,12 @@ func main() {
 		}
 
 		if !info.IsDir() {
-			b, err := os.ReadFile(path)
+			f, err := os.Open(path)
 			if err != nil {
 				return err
 			}
 
-			_, err = w.Write(b)
+			_, err = io.CopyBuffer(w, f, buff)
 			if err != nil {
 				return err
 			}
@@ -66,59 +78,72 @@ func main() {
 		log.Fatal(err)
 	}
 
-	f, err := os.OpenFile("zz_filesystem_generated.go", os.O_TRUNC|os.O_CREATE|os.O_WRONLY, 0644)
+	_, err = fmt.Fprint(srcOut, "}\n")
 	if err != nil {
 		log.Fatal(err)
 	}
-	defer f.Close()
+}
 
-	srcOut := bufio.NewWriter(f)
-	defer srcOut.Flush()
+// goByteArrayWriter dumps bytes as a Go integer hex literals separated by commas into underlying Writer.
+// Each line of the output will be indented by a tab and each line will contain at most 32 integer literals.
+// This is useful when generating Go array literals.
+type goByteArrayWriter struct {
+	i                 uint32
+	w                 io.Writer
+	hexDigitWithComma []byte
+}
 
-	_, err = fmt.Fprintf(srcOut, "// Code generated by go generate; DO NOT EDIT.\npackage function\n\nvar templatesZip = []byte{\n")
-	if err != nil {
-		log.Fatal(err)
+func newGoByteArrayWriter(w io.Writer) *goByteArrayWriter {
+	return &goByteArrayWriter{
+		i:                 0,
+		w:                 w,
+		hexDigitWithComma: []byte("0x00,"),
 	}
+}
 
-	zipBytesReader := bytes.NewReader(zipBuff.Bytes())
-	buff := make([]byte, 32)
-	for {
-		n, err := zipBytesReader.Read(buff)
-		if err != nil {
-			if errors.Is(err, io.EOF) {
-				break
-			} else {
-				log.Fatal(err)
+var hexs = []byte("0123456789abcdef")
+var space = []byte(" ")
+var newLine = []byte("\n")
+var tab = []byte("\t")
+
+const bytesInLine = 32
+
+func (g *goByteArrayWriter) Write(bs []byte) (written int, err error) {
+	var n int
+
+	for _, b := range bs {
+		if g.i == 0 {
+			n, err = g.w.Write(tab)
+			written += n
+			if err != nil {
+				return
 			}
 		}
 
-		_, err = fmt.Fprintf(srcOut, "\t")
+		g.hexDigitWithComma[2] = hexs[b>>4]
+		g.hexDigitWithComma[3] = hexs[b&0x0f]
+		n, err = g.w.Write(g.hexDigitWithComma)
+		written += n
 		if err != nil {
-			log.Fatal(err)
+			return
 		}
 
-		for i, b := range buff[:n] {
-			_, err = fmt.Fprintf(srcOut, "0x%02x,", b)
+		if g.i == bytesInLine-1 {
+			n, err = g.w.Write(newLine)
+			written += n
 			if err != nil {
-				log.Fatal(err)
+				return
 			}
-			if i < n-1 {
-				_, err = fmt.Fprintf(srcOut, " ")
-				if err != nil {
-					log.Fatal(err)
-				}
+			g.i = 0
+		} else {
+			n, err = g.w.Write(space)
+			written += n
+			if err != nil {
+				return
 			}
+			g.i++
 		}
-
-		_, err = fmt.Fprintf(srcOut, "\n")
-		if err != nil {
-			log.Fatal(err)
-		}
-
 	}
 
-	_, err = fmt.Fprintf(srcOut, "}\n")
-	if err != nil {
-		log.Fatal(err)
-	}
+	return
 }
diff --git a/invoke.go b/invoke.go
index 62f3bbc8..8e863b86 100644
--- a/invoke.go
+++ b/invoke.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"io"
 	"net/http"
 	"net/url"
 	"time"
@@ -45,13 +46,13 @@ func NewInvokeMessage() InvokeMessage {
 
 // invoke the Function instance in the target environment with the
 // invocation message.
-func invoke(ctx context.Context, c *Client, f Function, target string, m InvokeMessage) error {
+func invoke(ctx context.Context, c *Client, f Function, target string, m InvokeMessage) (string, error) {
 
 	// Get the first available route from 'local', 'remote', a named environment
 	// or treat target
 	route, err := invocationRoute(ctx, c, f, target) // choose instance to invoke
 	if err != nil {
-		return err
+		return "", err
 	}
 
 	// Format" either 'http' or 'cloudevent'
@@ -75,7 +76,7 @@ func invoke(ctx context.Context, c *Client, f Function, target string, m InvokeM
 	case "cloudevent":
 		return sendEvent(ctx, route, m, c.transport)
 	default:
-		return fmt.Errorf("format '%v' not supported.", format)
+		return "", fmt.Errorf("format '%v' not supported.", format)
 	}
 }
 
@@ -130,7 +131,7 @@ func invocationRoute(ctx context.Context, c *Client, f Function, target string)
 }
 
 // sendEvent to the route populated with data in the invoke message.
-func sendEvent(ctx context.Context, route string, m InvokeMessage, t http.RoundTripper) (err error) {
+func sendEvent(ctx context.Context, route string, m InvokeMessage, t http.RoundTripper) (resp string, err error) {
 	event := cloudevents.NewEvent()
 	event.SetID(m.ID)
 	event.SetSource(m.Source)
@@ -146,15 +147,18 @@ func sendEvent(ctx context.Context, route string, m InvokeMessage, t http.RoundT
 		return
 	}
 
-	result := c.Send(cloudevents.ContextWithTarget(ctx, route), event)
+	evt, result := c.Request(cloudevents.ContextWithTarget(ctx, route), event)
 	if cloudevents.IsUndelivered(result) {
 		err = fmt.Errorf("unable to invoke: %v", result)
+	} else if evt != nil { // Check for nil in case no event is returned
+		resp = evt.String()
 	}
+
 	return
 }
 
 // sendPost to the route populated with data in the invoke message.
-func sendPost(ctx context.Context, route string, m InvokeMessage, t http.RoundTripper) error {
+func sendPost(ctx context.Context, route string, m InvokeMessage, t http.RoundTripper) (string, error) {
 	client := http.Client{
 		Transport: t,
 		Timeout:   10 * time.Second,
@@ -167,11 +171,12 @@ func sendPost(ctx context.Context, route string, m InvokeMessage, t http.RoundTr
 		"Data":        {m.Data},
 	})
 	if err != nil {
-		return err
+		return "", err
 	}
 	defer resp.Body.Close()
-	if resp.StatusCode != 200 {
-		return fmt.Errorf("failure invoking '%v' (HTTP %v)", route, resp.StatusCode)
+	if resp.StatusCode > 299 {
+		return "", fmt.Errorf("failure invoking '%v' (HTTP %v)", route, resp.StatusCode)
 	}
-	return nil
+	b, err := io.ReadAll(resp.Body)
+	return string(b), err
 }


Test patch:
diff --git a/client_test.go b/client_test.go
index 94026d34..010f42de 100644
--- a/client_test.go
+++ b/client_test.go
@@ -1099,6 +1099,10 @@ func TestClient_Invoke_HTTP(t *testing.T) {
 		if req.Form.Get("ID") != message.ID {
 			t.Fatalf("expected message ID '%v', got '%v'", message.ID, req.Form.Get("ID"))
 		}
+		_, err := res.Write([]byte("hello world"))
+		if err != nil {
+			t.Fatal(err)
+		}
 	})
 
 	// Expose the masquarading Function on an OS-chosen port.
@@ -1139,10 +1143,16 @@ func TestClient_Invoke_HTTP(t *testing.T) {
 	defer job.Stop()
 
 	// Invoke the Function, which will use the mock Runner
-	if err := client.Invoke(context.Background(), f.Root, "", message); err != nil {
+	r, err := client.Invoke(context.Background(), f.Root, "", message)
+	if err != nil {
 		t.Fatal(err)
 	}
 
+	// Check the response value
+	if r != "hello world" {
+		t.Fatal("Unexpected response from function " + r)
+	}
+
 	// Fail if the Function was never invoked.
 	if !invoked {
 		t.Fatal("Function was not invoked")
@@ -1166,15 +1176,18 @@ func TestClient_Invoke_CloudEvent(t *testing.T) {
 		invoked bool // flag the Function was invoked
 		ctx     = context.Background()
 		message = fn.NewInvokeMessage() // message to send to the Function
+		evt     *cloudevents.Event      // A pointer to the received event
 	)
 
 	// A CloudEvent Receiver which masquarades as a running Function and
 	// verifies the invoker sent the message as a populated CloudEvent.
-	receiver := func(ctx context.Context, event cloudevents.Event) {
+	receiver := func(ctx context.Context, event cloudevents.Event) *cloudevents.Event {
 		invoked = true
 		if event.ID() != message.ID {
 			t.Fatalf("expected event ID '%v', got '%v'", message.ID, event.ID())
 		}
+		evt = &event
+		return evt
 	}
 
 	// A cloudevent receive handler which will expect the HTTP protocol
@@ -1224,10 +1237,14 @@ func TestClient_Invoke_CloudEvent(t *testing.T) {
 	defer job.Stop()
 
 	// Invoke the Function, which will use the mock Runner
-	if err := client.Invoke(context.Background(), f.Root, "", message); err != nil {
+	r, err := client.Invoke(context.Background(), f.Root, "", message)
+	if err != nil {
 		t.Fatal(err)
 	}
-
+	// Test the contents of the returned string.
+	if r != evt.String() {
+		t.Fatal("Invoke failed to return a response")
+	}
 	// Fail if the Function was never invoked.
 	if !invoked {
 		t.Fatal("Function was not invoked")