Merge pull request #1847 from semi-technologies/bugfix/gql-timestamps

Fix create/update timestamp issues in GraphQL and REST

adapters/handlers/graphql/local/get/class_builder.go

@@ -126,6 +126,8 @@ func (b *classBuilder) additionalFields(classProperties graphql.Fields, class *m
 	additionalProperties["certainty"] = b.additionalCertaintyField(class)
 	additionalProperties["vector"] = b.additionalVectorField(class)
 	additionalProperties["id"] = b.additionalIDField()
+	additionalProperties["creationTimeUnix"] = b.additionalCreationTimeUnix()
+	additionalProperties["lastUpdateTimeUnix"] = b.additionalLastUpdateTimeUnix()
 	// module specific additional properties
 	if b.modulesProvider != nil {
 		for name, field := range b.modulesProvider.GetAdditionalFields(class) {
@@ -173,3 +175,15 @@ func (b *classBuilder) additionalVectorField(class *models.Class) *graphql.Field
 		Type: graphql.NewList(graphql.Float),
 	}
 }
+
+func (b *classBuilder) additionalCreationTimeUnix() *graphql.Field {
+	return &graphql.Field{
+		Type: graphql.String,
+	}
+}
+
+func (b *classBuilder) additionalLastUpdateTimeUnix() *graphql.Field {
+	return &graphql.Field{
+		Type: graphql.String,
+	}
+}

adapters/handlers/graphql/local/get/class_builder_fields.go

@@ -422,7 +422,8 @@ type additionalCheck struct {
 }
 
 func (ac *additionalCheck) isAdditional(name string) bool {
-	if name == "classification" || name == "certainty" || name == "id" || name == "vector" {
+	if name == "classification" || name == "certainty" || name == "id" || name == "vector" ||
+		name == "creationTimeUnix" || name == "lastUpdateTimeUnix" {
 		return true
 	}
 	if ac.isModuleAdditional(name) {
@@ -499,6 +500,14 @@ func extractProperties(className string, selections *ast.SelectionSet,
 							additionalProps.Vector = true
 							continue
 						}
+						if additionalProperty == "creationTimeUnix" {
+							additionalProps.CreationTimeUnix = true
+							continue
+						}
+						if additionalProperty == "lastUpdateTimeUnix" {
+							additionalProps.LastUpdateTimeUnix = true
+							continue
+						}
 						if modulesProvider != nil {
 							if additionalCheck.isModuleAdditional(additionalProperty) {
 								additionalProps.ModuleParams = getModuleParams(additionalProps.ModuleParams)

adapters/handlers/graphql/local/get/get_test.go

@@ -14,7 +14,9 @@
 package get
 
 import (
+	"fmt"
 	"testing"
+	"time"
 
 	"github.com/go-openapi/strfmt"
 	"github.com/graphql-go/graphql/language/ast"
@@ -292,6 +294,9 @@ func TestExtractAdditionalFields(t *testing.T) {
 		expectedResult interface{}
 	}
 
+	// To facilitate testing timestamps
+	nowString := fmt.Sprint(time.Now().UnixNano() / int64(time.Millisecond))
+
 	tests := []test{
 		test{
 			name:  "with _additional certainty",
@@ -337,6 +342,50 @@ func TestExtractAdditionalFields(t *testing.T) {
 				},
 			},
 		},
+		test{
+			name:  "with _additional creationTimeUnix",
+			query: "{ Get { SomeAction { _additional { creationTimeUnix } } } }",
+			expectedParams: traverser.GetParams{
+				ClassName: "SomeAction",
+				AdditionalProperties: additional.Properties{
+					CreationTimeUnix: true,
+				},
+			},
+			resolverReturn: []interface{}{
+				map[string]interface{}{
+					"_additional": map[string]interface{}{
+						"creationTimeUnix": nowString,
+					},
+				},
+			},
+			expectedResult: map[string]interface{}{
+				"_additional": map[string]interface{}{
+					"creationTimeUnix": nowString,
+				},
+			},
+		},
+		test{
+			name:  "with _additional lastUpdateTimeUnix",
+			query: "{ Get { SomeAction { _additional { lastUpdateTimeUnix } } } }",
+			expectedParams: traverser.GetParams{
+				ClassName: "SomeAction",
+				AdditionalProperties: additional.Properties{
+					LastUpdateTimeUnix: true,
+				},
+			},
+			resolverReturn: []interface{}{
+				map[string]interface{}{
+					"_additional": map[string]interface{}{
+						"lastUpdateTimeUnix": nowString,
+					},
+				},
+			},
+			expectedResult: map[string]interface{}{
+				"_additional": map[string]interface{}{
+					"lastUpdateTimeUnix": nowString,
+				},
+			},
+		},
 		test{
 			name:  "with _additional classification",
 			query: "{ Get { SomeAction { _additional { classification { id completed classifiedFields scope basedOn }  } } } }",

entities/additional/classification.go

@@ -22,10 +22,12 @@ type Classification struct {
 }
 
 type Properties struct {
-	Classification bool                   `json:"classification"`
-	RefMeta        bool                   `json:"refMeta"`
-	Vector         bool                   `json:"vector"`
-	Certainty      bool                   `json:"certainty"`
-	ID             bool                   `json:"id"`
-	ModuleParams   map[string]interface{} `json:"moduleParams"`
+	Classification     bool                   `json:"classification"`
+	RefMeta            bool                   `json:"refMeta"`
+	Vector             bool                   `json:"vector"`
+	Certainty          bool                   `json:"certainty"`
+	ID                 bool                   `json:"id"`
+	CreationTimeUnix   bool                   `json:"creationTimeUnix"`
+	LastUpdateTimeUnix bool                   `json:"lastUpdateTimeUnix"`
+	ModuleParams       map[string]interface{} `json:"moduleParams"`
 }

tools/dev/run_dev_server.sh

@@ -215,6 +215,20 @@ case $CONFIG in
         --write-timeout=600s
     ;;
 
+  local-no-modules)
+      QUERY_DEFAULTS_LIMIT=20 \
+      ORIGIN=http://localhost:8080 \
+      AUTHENTICATION_ANONYMOUS_ACCESS_ENABLED=true \
+      DEFAULT_VECTORIZER_MODULE=none \
+      PERSISTENCE_DATA_PATH="./data" \
+      go run ./cmd/weaviate-server \
+        --scheme http \
+        --host "127.0.0.1" \
+        --port 8080 \
+        --read-timeout=3600s \
+        --write-timeout=3600s
+    ;;
+
   *) 
     echo "Invalid config" 2>&1
     exit 1

usecases/objects/update.go

@@ -62,6 +62,11 @@ func (m *Manager) updateObjectToConnectorAndSchema(ctx context.Context, principa
 		return nil, NewErrInvalidUserInput("invalid object: %v", err)
 	}
 
+	// Set the original creation timestamp before call to put,
+	// otherwise it is lost. This is because `class` is unmarshaled
+	// directly from the request body, therefore `CreationTimeUnix`
+	// inherits the zero value.
+	class.CreationTimeUnix = originalObject.Created
 	class.LastUpdateTimeUnix = m.timeSource.Now()
 
 	err = m.vectorizeAndPutObject(ctx, class, principal)

usecases/objects/update_test.go

@@ -0,0 +1,106 @@
+package objects
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/go-openapi/strfmt"
+	"github.com/semi-technologies/weaviate/adapters/repos/db/vector/hnsw"
+	"github.com/semi-technologies/weaviate/entities/models"
+	"github.com/semi-technologies/weaviate/entities/schema"
+	"github.com/semi-technologies/weaviate/entities/search"
+	"github.com/semi-technologies/weaviate/usecases/config"
+	"github.com/sirupsen/logrus/hooks/test"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+)
+
+func Test_UpdateAction(t *testing.T) {
+	var (
+		db            *fakeVectorRepo
+		vectorizer    *fakeVectorizer
+		manager       *Manager
+		extender      *fakeExtender
+		projectorFake *fakeProjector
+	)
+
+	schema := schema.Schema{
+		Objects: &models.Schema{
+			Classes: []*models.Class{
+				{
+					Class:             "ActionClass",
+					VectorIndexConfig: hnsw.NewDefaultUserConfig(),
+					Properties: []*models.Property{
+						{
+							DataType: []string{"string"},
+							Name:     "foo",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	reset := func() {
+		db = &fakeVectorRepo{}
+		schemaManager := &fakeSchemaManager{
+			GetSchemaResponse: schema,
+		}
+		locks := &fakeLocks{}
+		cfg := &config.WeaviateConfig{}
+		cfg.Config.QueryDefaults.Limit = 20
+		cfg.Config.QueryMaximumResults = 200
+		authorizer := &fakeAuthorizer{}
+		logger, _ := test.NewNullLogger()
+		extender = &fakeExtender{}
+		projectorFake = &fakeProjector{}
+		vectorizer = &fakeVectorizer{}
+		vecProvider := &fakeVectorizerProvider{vectorizer}
+		manager = NewManager(locks, schemaManager, cfg, logger, authorizer,
+			vecProvider, db, getFakeModulesProviderWithCustomExtenders(extender, projectorFake))
+	}
+
+	t.Run("ensure creation timestamp persists", func(t *testing.T) {
+		reset()
+
+		beforeUpdate := time.Now().UnixNano() / int64(time.Millisecond)
+		id := strfmt.UUID("34e9df15-0c3b-468d-ab99-f929662834c7")
+		vec := []float32{0, 1, 2}
+
+		result := &search.Result{
+			ID:        id,
+			ClassName: "ActionClass",
+			Schema:    map[string]interface{}{"foo": "bar"},
+			Created:   beforeUpdate,
+			Updated:   beforeUpdate,
+		}
+		db.On("ObjectByID", id, mock.Anything, mock.Anything).Return(result, nil).Once()
+		vectorizer.On("UpdateObject", mock.Anything).Return(vec, nil).Once()
+		db.On("PutObject", mock.Anything, mock.Anything).Return(nil).Once()
+
+		payload := &models.Object{
+			Class:      "ActionClass",
+			ID:         id,
+			Properties: map[string]interface{}{"foo": "baz"},
+		}
+		res, err := manager.UpdateObject(context.Background(), &models.Principal{}, id, payload)
+		require.Nil(t, err)
+		expected := &models.Object{
+			Class:            "ActionClass",
+			ID:               id,
+			Properties:       map[string]interface{}{"foo": "baz"},
+			CreationTimeUnix: beforeUpdate,
+		}
+
+		afterUpdate := time.Now().UnixNano() / int64(time.Millisecond)
+
+		assert.Equal(t, expected.Class, res.Class)
+		assert.Equal(t, expected.ID, res.ID)
+		assert.Equal(t, expected.Properties, res.Properties)
+		assert.Equal(t, expected.CreationTimeUnix, res.CreationTimeUnix)
+		assert.GreaterOrEqual(t, res.LastUpdateTimeUnix, beforeUpdate)
+		assert.LessOrEqual(t, res.LastUpdateTimeUnix, afterUpdate)
+	})
+}

usecases/traverser/explorer.go

@@ -260,6 +260,14 @@ func (e *Explorer) searchResultsToGetResponse(ctx context.Context,
 			additionalProperties["vector"] = res.Vector
 		}
 
+		if params.AdditionalProperties.CreationTimeUnix {
+			additionalProperties["creationTimeUnix"] = res.Created
+		}
+
+		if params.AdditionalProperties.LastUpdateTimeUnix {
+			additionalProperties["lastUpdateTimeUnix"] = res.Updated
+		}
+
 		if len(additionalProperties) > 0 {
 			res.Schema.(map[string]interface{})["_additional"] = additionalProperties
 		}