api: dont create tag on chunk upload (#1138)

pkg/api/api.go

@@ -140,12 +140,16 @@ func (s *server) getOrCreateTag(tagUid string) (*tags.Tag, bool, error) {
 		}
 		return tag, true, nil
 	}
+	t, err := s.getTag(tagUid)
+	return t, false, err
+}
+
+func (s *server) getTag(tagUid string) (*tags.Tag, error) {
 	uid, err := strconv.Atoi(tagUid)
 	if err != nil {
-		return nil, false, fmt.Errorf("cannot parse taguid: %w", err)
+		return nil, fmt.Errorf("cannot parse taguid: %w", err)
 	}
-	t, err := s.Tags.Get(uint32(uid))
-	return t, false, err
+	return s.Tags.Get(uint32(uid))
 }
 
 func (s *server) resolveNameOrAddress(str string) (swarm.Address, error) {

pkg/api/chunk.go

@@ -35,24 +35,32 @@ func (s *server) chunkUploadHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	tag, _, err := s.getOrCreateTag(r.Header.Get(SwarmTagUidHeader))
-	if err != nil {
-		s.Logger.Debugf("chunk upload: get or create tag: %v", err)
-		s.Logger.Error("chunk upload: get or create tag")
-		jsonhttp.InternalServerError(w, "cannot get or create tag")
-		return
-	}
+	var (
+		tag *tags.Tag
+		ctx = r.Context()
+	)
 
-	// Add the tag to the context
-	ctx := sctx.SetTag(r.Context(), tag)
+	if h := r.Header.Get(SwarmTagUidHeader); h != "" {
+		tag, err = s.getTag(h)
+		if err != nil {
+			s.Logger.Debugf("chunk upload: get tag: %v", err)
+			s.Logger.Error("chunk upload: get tag")
+			jsonhttp.BadRequest(w, "cannot get tag")
+			return
 
-	// Increment the StateSplit here since we dont have a splitter for the file upload
-	err = tag.Inc(tags.StateSplit)
-	if err != nil {
-		s.Logger.Debugf("chunk upload: increment tag: %v", err)
-		s.Logger.Error("chunk upload: increment tag")
-		jsonhttp.InternalServerError(w, "increment tag")
-		return
+		}
+
+		// add the tag to the context if it exists
+		ctx = sctx.SetTag(r.Context(), tag)
+
+		// increment the StateSplit here since we dont have a splitter for the file upload
+		err = tag.Inc(tags.StateSplit)
+		if err != nil {
+			s.Logger.Debugf("chunk upload: increment tag: %v", err)
+			s.Logger.Error("chunk upload: increment tag")
+			jsonhttp.InternalServerError(w, "increment tag")
+			return
+		}
 	}
 
 	data, err := ioutil.ReadAll(r.Body)
@@ -82,7 +90,7 @@ func (s *server) chunkUploadHandler(w http.ResponseWriter, r *http.Request) {
 		s.Logger.Error("chunk upload: chunk write error")
 		jsonhttp.BadRequest(w, "chunk write error")
 		return
-	} else if len(seen) > 0 && seen[0] {
+	} else if len(seen) > 0 && seen[0] && tag != nil {
 		err := tag.Inc(tags.StateSeen)
 		if err != nil {
 			s.Logger.Debugf("chunk upload: increment tag", err)
@@ -92,16 +100,18 @@ func (s *server) chunkUploadHandler(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
-	// Indicate that the chunk is stored
-	err = tag.Inc(tags.StateStored)
-	if err != nil {
-		s.Logger.Debugf("chunk upload: increment tag", err)
-		s.Logger.Error("chunk upload: increment tag")
-		jsonhttp.BadRequest(w, "increment tag")
-		return
+	if tag != nil {
+		// indicate that the chunk is stored
+		err = tag.Inc(tags.StateStored)
+		if err != nil {
+			s.Logger.Debugf("chunk upload: increment tag", err)
+			s.Logger.Error("chunk upload: increment tag")
+			jsonhttp.InternalServerError(w, "increment tag")
+			return
+		}
+		w.Header().Set(SwarmTagUidHeader, fmt.Sprint(tag.Uid))
 	}
 
-	w.Header().Set(SwarmTagUidHeader, fmt.Sprint(tag.Uid))
 	w.Header().Set("Access-Control-Expose-Headers", SwarmTagUidHeader)
 	jsonhttp.OK(w, nil)
 }

pkg/api/router.go

@@ -111,18 +111,18 @@ func (s *server) setupRouting() {
 		web.FinalHandler(jsonhttp.MethodHandler{
 			"POST": web.ChainHandlers(
 				jsonhttp.NewMaxBodyBytesHandler(1024),
-				web.FinalHandlerFunc(s.createTag),
+				web.FinalHandlerFunc(s.createTagHandler),
 			),
 		})),
 	)
 	handle(router, "/tags/{id}", web.ChainHandlers(
 		s.gatewayModeForbidEndpointHandler,
 		web.FinalHandler(jsonhttp.MethodHandler{
-			"GET":    http.HandlerFunc(s.getTag),
-			"DELETE": http.HandlerFunc(s.deleteTag),
+			"GET":    http.HandlerFunc(s.getTagHandler),
+			"DELETE": http.HandlerFunc(s.deleteTagHandler),
 			"PATCH": web.ChainHandlers(
 				jsonhttp.NewMaxBodyBytesHandler(1024),
-				web.FinalHandlerFunc(s.doneSplit),
+				web.FinalHandlerFunc(s.doneSplitHandler),
 			),
 		})),
 	)

pkg/api/tag.go

@@ -52,7 +52,7 @@ func newTagResponse(tag *tags.Tag) tagResponse {
 	}
 }
 
-func (s *server) createTag(w http.ResponseWriter, r *http.Request) {
+func (s *server) createTagHandler(w http.ResponseWriter, r *http.Request) {
 	body, err := ioutil.ReadAll(r.Body)
 	if err != nil {
 		if jsonhttp.HandleBodyReadError(err, w) {
@@ -90,7 +90,7 @@ func (s *server) createTag(w http.ResponseWriter, r *http.Request) {
 	jsonhttp.Created(w, newTagResponse(tag))
 }
 
-func (s *server) getTag(w http.ResponseWriter, r *http.Request) {
+func (s *server) getTagHandler(w http.ResponseWriter, r *http.Request) {
 	idStr := mux.Vars(r)["id"]
 
 	id, err := strconv.Atoi(idStr)
@@ -119,7 +119,7 @@ func (s *server) getTag(w http.ResponseWriter, r *http.Request) {
 	jsonhttp.OK(w, newTagResponse(tag))
 }
 
-func (s *server) deleteTag(w http.ResponseWriter, r *http.Request) {
+func (s *server) deleteTagHandler(w http.ResponseWriter, r *http.Request) {
 	idStr := mux.Vars(r)["id"]
 
 	id, err := strconv.Atoi(idStr)
@@ -148,7 +148,7 @@ func (s *server) deleteTag(w http.ResponseWriter, r *http.Request) {
 	jsonhttp.NoContent(w)
 }
 
-func (s *server) doneSplit(w http.ResponseWriter, r *http.Request) {
+func (s *server) doneSplitHandler(w http.ResponseWriter, r *http.Request) {
 	idStr := mux.Vars(r)["id"]
 
 	id, err := strconv.Atoi(idStr)

pkg/api/tag_test.go

@@ -19,7 +19,6 @@ import (
 	"github.com/ethersphere/bee/pkg/api"
 	"github.com/ethersphere/bee/pkg/jsonhttp"
 	"github.com/ethersphere/bee/pkg/jsonhttp/jsonhttptest"
-	mp "github.com/ethersphere/bee/pkg/pusher/mock"
 	"github.com/ethersphere/bee/pkg/storage/mock"
 	testingc "github.com/ethersphere/bee/pkg/storage/testing"
 	"github.com/ethersphere/bee/pkg/swarm"
@@ -46,7 +45,6 @@ func TestTags(t *testing.T) {
 		mockStatestore = statestore.NewStateStore()
 		logger         = logging.New(ioutil.Discard, 0)
 		tag            = tags.NewTags(mockStatestore, logger)
-		mockPusher     = mp.NewMockPusher(tag)
 		client, _, _   = newTestServer(t, testServerOptions{
 			Storer: mock.NewStorer(),
 			Tags:   tag,
@@ -80,11 +78,11 @@ func TestTags(t *testing.T) {
 	})
 
 	t.Run("create tag with invalid id", func(t *testing.T) {
-		jsonhttptest.Request(t, client, http.MethodPost, chunksResource(chunk.Address()), http.StatusInternalServerError,
+		jsonhttptest.Request(t, client, http.MethodPost, chunksResource(chunk.Address()), http.StatusBadRequest,
 			jsonhttptest.WithRequestBody(bytes.NewReader(chunk.Data())),
 			jsonhttptest.WithExpectedJSONResponse(jsonhttp.StatusResponse{
-				Message: "cannot get or create tag",
-				Code:    http.StatusInternalServerError,
+				Message: "cannot get tag",
+				Code:    http.StatusBadRequest,
 			}),
 			jsonhttptest.WithRequestHeader(api.SwarmTagUidHeader, "invalid_id.jpg"), // the value should be uint32
 		)
@@ -108,18 +106,6 @@ func TestTags(t *testing.T) {
 		)
 	})
 
-	t.Run("tag id in chunk upload", func(t *testing.T) {
-		rcvdHeaders := jsonhttptest.Request(t, client, http.MethodPost, chunksResource(chunk.Address()), http.StatusOK,
-			jsonhttptest.WithRequestBody(bytes.NewReader(chunk.Data())),
-			jsonhttptest.WithExpectedJSONResponse(jsonhttp.StatusResponse{
-				Message: http.StatusText(http.StatusOK),
-				Code:    http.StatusOK,
-			}),
-		)
-
-		isTagFoundInResponse(t, rcvdHeaders, nil)
-	})
-
 	t.Run("create tag upload chunk", func(t *testing.T) {
 		// create a tag using the API
 		tr := api.TagResponse{}
@@ -134,44 +120,25 @@ func TestTags(t *testing.T) {
 			t.Fatalf("sent tag name %s does not match received tag name %s", someTagName, tr.Name)
 		}
 
-		// now upload a chunk and see if we receive a tag with the same id
-		rcvdHeaders := jsonhttptest.Request(t, client, http.MethodPost, chunksResource(chunk.Address()), http.StatusOK,
+		_ = jsonhttptest.Request(t, client, http.MethodPost, chunksResource(chunk.Address()), http.StatusOK,
 			jsonhttptest.WithRequestBody(bytes.NewReader(chunk.Data())),
 			jsonhttptest.WithExpectedJSONResponse(jsonhttp.StatusResponse{
 				Message: http.StatusText(http.StatusOK),
 				Code:    http.StatusOK,
 			}),
-			jsonhttptest.WithRequestHeader(api.SwarmTagUidHeader, strconv.FormatUint(uint64(tr.Uid), 10)),
 		)
 
-		isTagFoundInResponse(t, rcvdHeaders, &tr)
-		tagValueTest(t, tr.Uid, 1, 1, 1, 0, 0, 0, swarm.ZeroAddress, client)
-	})
-
-	t.Run("tag counters", func(t *testing.T) {
 		rcvdHeaders := jsonhttptest.Request(t, client, http.MethodPost, chunksResource(chunk.Address()), http.StatusOK,
 			jsonhttptest.WithRequestBody(bytes.NewReader(chunk.Data())),
 			jsonhttptest.WithExpectedJSONResponse(jsonhttp.StatusResponse{
 				Message: http.StatusText(http.StatusOK),
 				Code:    http.StatusOK,
 			}),
+			jsonhttptest.WithRequestHeader(api.SwarmTagUidHeader, strconv.FormatUint(uint64(tr.Uid), 10)),
 		)
-		id := isTagFoundInResponse(t, rcvdHeaders, nil)
-
-		tag, err := tag.Get(id)
-		if err != nil {
-			t.Fatal(err)
-		}
-		err = mockPusher.SendChunk(id)
-		if err != nil {
-			t.Fatal(err)
-		}
-		err = mockPusher.RcvdReceipt(id)
-		if err != nil {
-			t.Fatal(err)
-		}
 
-		tagValueTest(t, id, tag.Split, tag.Stored, tag.Seen, tag.Sent, tag.Synced, tag.Total, swarm.ZeroAddress, client)
+		isTagFoundInResponse(t, rcvdHeaders, &tr)
+		tagValueTest(t, tr.Uid, 1, 1, 1, 0, 0, 0, swarm.ZeroAddress, client)
 	})
 
 	t.Run("delete tag error", func(t *testing.T) {
@@ -376,6 +343,8 @@ func TestTags(t *testing.T) {
 // isTagFoundInResponse verifies that the tag id is found in the supplied HTTP headers
 // if an API tag response is supplied, it also verifies that it contains an id which matches the headers
 func isTagFoundInResponse(t *testing.T, headers http.Header, tr *api.TagResponse) uint32 {
+	t.Helper()
+
 	idStr := headers.Get(api.SwarmTagUidHeader)
 	if idStr == "" {
 		t.Fatalf("could not find tag id header in chunk upload response")