feat(search): confirm the motion photo video via tika instead of reading bytes

services/search/pkg/content/cs3.go

@@ -93,64 +93,3 @@ func (s cs3) Retrieve(ctx context.Context, rID *provider.ResourceId) (io.ReadClo
 
 	return cres.Body, nil
 }
-
-// RetrieveRange downloads length bytes starting at offset from a cs3 service.
-// The caller MUST make sure to close the returned ReadCloser.
-// It relies on HTTP range support of the download endpoint. If the endpoint
-// ignores the Range header and returns the full file (200 instead of 206), the
-// leading offset bytes are discarded so the returned reader is always positioned
-// at offset.
-func (s cs3) RetrieveRange(ctx context.Context, rID *provider.ResourceId, offset, length int64) (io.ReadCloser, error) {
-	if offset < 0 || length <= 0 {
-		return nil, fmt.Errorf("invalid range: offset %d, length %d", offset, length)
-	}
-
-	ep, tt, at, err := s.initiateDownload(ctx, rID)
-	if err != nil {
-		return nil, err
-	}
-
-	req, err := http.NewRequest(http.MethodGet, ep, nil)
-	if err != nil {
-		return nil, err
-	}
-
-	req.Header.Set(revactx.TokenHeader, at)
-	req.Header.Set("X-Reva-Transfer", tt)
-	// A single range keeps the response a plain 206 with a Content-Range header,
-	// never a multipart/byteranges body.
-	req.Header.Set("Range", fmt.Sprintf("bytes=%d-%d", offset, offset+length-1))
-
-	cres, err := s.httpClient.Do(req)
-	if err != nil {
-		return nil, err
-	}
-
-	switch cres.StatusCode {
-	case http.StatusPartialContent:
-		// Range honored: the body already starts at offset.
-		return capped(cres.Body, length), nil
-	case http.StatusOK:
-		// Range ignored: the body is the whole file. Skip to offset so the
-		// caller always reads from the requested position.
-		if _, err := io.CopyN(io.Discard, cres.Body, offset); err != nil {
-			_ = cres.Body.Close()
-			return nil, fmt.Errorf("could not skip to offset %d: %w", offset, err)
-		}
-		return capped(cres.Body, length), nil
-	default:
-		_ = cres.Body.Close()
-		return nil, fmt.Errorf("could not download range. Request returned with statuscode %d ", cres.StatusCode)
-	}
-}
-
-type cappedReadCloser struct {
-	io.Reader
-	io.Closer
-}
-
-// capped limits rc to length bytes, keeping the contract even when the server
-// returns more than the requested range.
-func capped(rc io.ReadCloser, length int64) io.ReadCloser {
-	return cappedReadCloser{io.LimitReader(rc, length), rc}
-}

services/search/pkg/content/mocks/retriever.go

@@ -106,83 +106,3 @@ func (_c *Retriever_Retrieve_Call) RunAndReturn(run func(ctx context.Context, rI
 	_c.Call.Return(run)
 	return _c
 }
-
-// RetrieveRange provides a mock function for the type Retriever
-func (_mock *Retriever) RetrieveRange(ctx context.Context, rID *providerv1beta1.ResourceId, offset int64, length int64) (io.ReadCloser, error) {
-	ret := _mock.Called(ctx, rID, offset, length)
-
-	if len(ret) == 0 {
-		panic("no return value specified for RetrieveRange")
-	}
-
-	var r0 io.ReadCloser
-	var r1 error
-	if returnFunc, ok := ret.Get(0).(func(context.Context, *providerv1beta1.ResourceId, int64, int64) (io.ReadCloser, error)); ok {
-		return returnFunc(ctx, rID, offset, length)
-	}
-	if returnFunc, ok := ret.Get(0).(func(context.Context, *providerv1beta1.ResourceId, int64, int64) io.ReadCloser); ok {
-		r0 = returnFunc(ctx, rID, offset, length)
-	} else {
-		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(io.ReadCloser)
-		}
-	}
-	if returnFunc, ok := ret.Get(1).(func(context.Context, *providerv1beta1.ResourceId, int64, int64) error); ok {
-		r1 = returnFunc(ctx, rID, offset, length)
-	} else {
-		r1 = ret.Error(1)
-	}
-	return r0, r1
-}
-
-// Retriever_RetrieveRange_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'RetrieveRange'
-type Retriever_RetrieveRange_Call struct {
-	*mock.Call
-}
-
-// RetrieveRange is a helper method to define mock.On call
-//   - ctx context.Context
-//   - rID *providerv1beta1.ResourceId
-//   - offset int64
-//   - length int64
-func (_e *Retriever_Expecter) RetrieveRange(ctx interface{}, rID interface{}, offset interface{}, length interface{}) *Retriever_RetrieveRange_Call {
-	return &Retriever_RetrieveRange_Call{Call: _e.mock.On("RetrieveRange", ctx, rID, offset, length)}
-}
-
-func (_c *Retriever_RetrieveRange_Call) Run(run func(ctx context.Context, rID *providerv1beta1.ResourceId, offset int64, length int64)) *Retriever_RetrieveRange_Call {
-	_c.Call.Run(func(args mock.Arguments) {
-		var arg0 context.Context
-		if args[0] != nil {
-			arg0 = args[0].(context.Context)
-		}
-		var arg1 *providerv1beta1.ResourceId
-		if args[1] != nil {
-			arg1 = args[1].(*providerv1beta1.ResourceId)
-		}
-		var arg2 int64
-		if args[2] != nil {
-			arg2 = args[2].(int64)
-		}
-		var arg3 int64
-		if args[3] != nil {
-			arg3 = args[3].(int64)
-		}
-		run(
-			arg0,
-			arg1,
-			arg2,
-			arg3,
-		)
-	})
-	return _c
-}
-
-func (_c *Retriever_RetrieveRange_Call) Return(readCloser io.ReadCloser, err error) *Retriever_RetrieveRange_Call {
-	_c.Call.Return(readCloser, err)
-	return _c
-}
-
-func (_c *Retriever_RetrieveRange_Call) RunAndReturn(run func(ctx context.Context, rID *providerv1beta1.ResourceId, offset int64, length int64) (io.ReadCloser, error)) *Retriever_RetrieveRange_Call {
-	_c.Call.Return(run)
-	return _c
-}

services/search/pkg/content/retriever.go

@@ -12,10 +12,6 @@ import (
 // It requests and then returns a resource from the underlying storage.
 type Retriever interface {
 	Retrieve(ctx context.Context, rID *provider.ResourceId) (io.ReadCloser, error)
-	// RetrieveRange returns a reader positioned at offset for up to length bytes
-	// of the resource. Implementations must ensure the reader starts at offset
-	// even when the storage does not honor HTTP range requests.
-	RetrieveRange(ctx context.Context, rID *provider.ResourceId, offset, length int64) (io.ReadCloser, error)
 }
 
 func contextGet(ctx context.Context, k string) (string, bool) {

services/search/pkg/content/tika.go

@@ -82,6 +82,7 @@ func (t Tika) Extract(ctx context.Context, ri *provider.ResourceInfo) (Document,
 		return doc, err
 	}
 
+	var motionPhotoVideo bool
 	for _, meta := range metas {
 		title, err := getFirstValue(meta, "dc:title")
 		if err != nil {
@@ -118,11 +119,14 @@ func (t Tika) Extract(ctx context.Context, ri *provider.ResourceInfo) (Document,
 		if v := t.getMotionPhoto(meta); v != nil {
 			doc.MotionPhoto = v
 		}
+		if isMotionPhotoVideo(meta) {
+			motionPhotoVideo = true
+		}
 	}
 
-	// verify against the file itself: a shared motion photo can keep the XMP but
-	// lose the appended video, which would leave an unplayable facet.
-	if doc.MotionPhoto != nil && !t.motionPhotoHasVideo(ctx, ri, doc.MotionPhoto.GetVideoSize()) {
+	// the xmp alone does not prove the video is there, tika emitting it as an
+	// embedded attachment does
+	if !motionPhotoVideo {
 		doc.MotionPhoto = nil
 	}
 

services/search/pkg/content/tika_motion_photo.go

@@ -1,20 +1,16 @@
 package content
 
 import (
-	"context"
-	"io"
+	libregraph "github.com/opencloud-eu/libre-graph-api-go"
 	"sort"
 	"strconv"
 	"strings"
-
-	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
-	libregraph "github.com/opencloud-eu/libre-graph-api-go"
 )
 
-// motionPhotoVideoSignatureLen is the number of trailing bytes we read to confirm
-// an actual video is present. Enough to cover the ISO base media (MP4) box size
-// and the "ftyp" box type at bytes [4:8].
-const motionPhotoVideoSignatureLen = 12
+// motionPhotoVideoName is the resource name tika gives the appended video when
+// it emits it as an embedded attachment. The extension follows the detected
+// type and is absent for legacy MicroVideo, which declares no mime type.
+const motionPhotoVideoName = "motion-photo"
 
 // getMotionPhoto reads Google Motion Photo XMP, which Tika exposes under the
 // canonical Camera/Container prefixes. It covers both the current MotionPhoto
@@ -89,31 +85,15 @@ func motionPhotoVideoSize(meta map[string][]string) (int64, bool) {
 	return 0, false
 }
 
-// looksLikeMP4 reports whether buf begins with an ISO base media (MP4/QuickTime)
-// "ftyp" box, which Google Motion Photo and legacy MicroVideo clips start with.
-func looksLikeMP4(buf []byte) bool {
-	return len(buf) >= 8 && string(buf[4:8]) == "ftyp"
-}
-
-// motionPhotoHasVideo confirms that the file actually contains the embedded video
-// the XMP advertises. A photos.google.com share strips the appended video but
-// keeps the XMP, which would otherwise make us expose an unplayable facet. The
-// video is appended at the end, so it starts at fileSize-videoSize; we read a few
-// bytes there and require an MP4 signature.
-func (t Tika) motionPhotoHasVideo(ctx context.Context, ri *provider.ResourceInfo, videoSize int64) bool {
-	size := int64(ri.GetSize())
-	if videoSize <= 0 || videoSize >= size {
-		return false
-	}
-
-	rc, err := t.RetrieveRange(ctx, ri.GetId(), size-videoSize, motionPhotoVideoSignatureLen)
+// isMotionPhotoVideo reports whether meta describes the video tika extracted
+// from a motion photo. Tika only emits it when the bytes the xmp advertises are
+// really there, so its presence is what confirms the facet: a shared motion
+// photo can keep the xmp and lose the appended video.
+func isMotionPhotoVideo(meta map[string][]string) bool {
+	// tika 4 renamed the meta prefix from X-TIKA: to tk:
+	name, err := getFirstValue(meta, "tk:resource-name", "X-TIKA:resource-name")
 	if err != nil {
-		t.logger.Debug().Err(err).Interface("ResourceID", ri.GetId()).Msg("could not read motion photo video header, dropping facet")
 		return false
 	}
-	defer rc.Close()
-
-	buf := make([]byte, motionPhotoVideoSignatureLen)
-	n, _ := io.ReadFull(rc, buf)
-	return looksLikeMP4(buf[:n])
+	return name == motionPhotoVideoName || strings.HasPrefix(name, motionPhotoVideoName+".")
 }

services/search/pkg/content/tika_motion_photo_test.go

@@ -1,17 +1,9 @@
 package content
 
 import (
-	"context"
-	"errors"
-	"io"
-	"strings"
-
-	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	libregraph "github.com/opencloud-eu/libre-graph-api-go"
-
-	"github.com/opencloud-eu/opencloud/pkg/log"
 )
 
 var _ = Describe("getMotionPhoto", func() {
@@ -85,73 +77,16 @@ var _ = Describe("getMotionPhoto", func() {
 	})
 })
 
-var _ = Describe("looksLikeMP4", func() {
-	It("recognizes an ftyp box and rejects everything else", func() {
-		Expect(looksLikeMP4([]byte{0, 0, 0, 24, 'f', 't', 'y', 'p', 'i', 's', 'o', 'm'})).To(BeTrue())
-		Expect(looksLikeMP4([]byte("JFIF garbage"))).To(BeFalse())
-		Expect(looksLikeMP4([]byte{0, 0, 0})).To(BeFalse())
-	})
-})
-
-// rangeStub serves RetrieveRange from a string and records the requested range.
-type rangeStub struct {
-	data      string
-	err       error
-	gotOffset int64
-	gotLength int64
-	calls     int
-}
-
-func (r *rangeStub) Retrieve(context.Context, *provider.ResourceId) (io.ReadCloser, error) {
-	return nil, errors.New("unused")
-}
-
-func (r *rangeStub) RetrieveRange(_ context.Context, _ *provider.ResourceId, offset, length int64) (io.ReadCloser, error) {
-	r.calls++
-	r.gotOffset, r.gotLength = offset, length
-	if r.err != nil {
-		return nil, r.err
-	}
-	return io.NopCloser(strings.NewReader(r.data)), nil
-}
-
-var _ = Describe("motionPhotoHasVideo", func() {
-	var (
-		retriever *rangeStub
-		tika      Tika
-		ri        *provider.ResourceInfo
+var _ = Describe("isMotionPhotoVideo", func() {
+	DescribeTable("recognizes the video tika emits as an embedded attachment",
+		func(meta map[string][]string, expected bool) {
+			Expect(isMotionPhotoVideo(meta)).To(Equal(expected))
+		},
+		Entry("named attachment", map[string][]string{"tk:resource-name": {"motion-photo.mp4"}}, true),
+		Entry("legacy tika prefix", map[string][]string{"X-TIKA:resource-name": {"motion-photo.mp4"}}, true),
+		Entry("no extension, as for MicroVideo", map[string][]string{"tk:resource-name": {"motion-photo"}}, true),
+		Entry("another attachment", map[string][]string{"tk:resource-name": {"cover.jpg"}}, false),
+		Entry("a name that only starts alike", map[string][]string{"tk:resource-name": {"motion-photography.mp4"}}, false),
+		Entry("the image itself", map[string][]string{"Camera:MotionPhoto": {"1"}}, false),
 	)
-
-	BeforeEach(func() {
-		retriever = &rangeStub{}
-		basic, err := NewBasicExtractor(log.NewLogger())
-		Expect(err).ToNot(HaveOccurred())
-		tika = Tika{Basic: basic, Retriever: retriever}
-		ri = &provider.ResourceInfo{Size: 100}
-	})
-
-	It("confirms a video that starts with an ftyp box", func() {
-		retriever.data = "\x00\x00\x00\x18ftypisom"
-		Expect(tika.motionPhotoHasVideo(context.Background(), ri, 40)).To(BeTrue())
-		Expect(retriever.gotOffset).To(Equal(int64(60)), "the video starts at size-videoSize")
-		Expect(retriever.gotLength).To(Equal(int64(motionPhotoVideoSignatureLen)))
-	})
-
-	It("rejects trailing bytes without an MP4 signature", func() {
-		retriever.data = "not a video "
-		Expect(tika.motionPhotoHasVideo(context.Background(), ri, 40)).To(BeFalse())
-	})
-
-	It("rejects degenerate video sizes without reading", func() {
-		Expect(tika.motionPhotoHasVideo(context.Background(), ri, 0)).To(BeFalse())
-		Expect(tika.motionPhotoHasVideo(context.Background(), ri, -1)).To(BeFalse())
-		Expect(tika.motionPhotoHasVideo(context.Background(), ri, 100)).To(BeFalse())
-		Expect(tika.motionPhotoHasVideo(context.Background(), ri, 101)).To(BeFalse())
-		Expect(retriever.calls).To(BeZero())
-	})
-
-	It("drops the facet when the range read fails", func() {
-		retriever.err = errors.New("nope")
-		Expect(tika.motionPhotoHasVideo(context.Background(), ri, 40)).To(BeFalse())
-	})
 })

services/search/pkg/content/tika_test.go

@@ -217,13 +217,8 @@ var _ = Describe("Tika", func() {
 			Expect(doc.Content).To(Equal("one two"))
 		})
 
-		It("verifies the motion photo video against the file", func() {
-			fullResponse = `[{"Camera:MotionPhotoVersion": "1", "Container:Directory/Item[2]/Item:Semantic": "MotionPhoto", "Container:Directory/Item[2]/Item:Length": "40"}]`
-			retriever := &contentMocks.Retriever{}
-			retriever.On("Retrieve", mock.Anything, mock.Anything).Return(io.NopCloser(strings.NewReader("")), nil)
-			retriever.On("RetrieveRange", mock.Anything, mock.Anything, int64(60), mock.Anything).
-				Return(io.NopCloser(strings.NewReader("\x00\x00\x00\x18ftypisom")), nil)
-			tika.Retriever = retriever
+		It("keeps the motion photo facet when tika emits the video", func() {
+			fullResponse = `[{"Camera:MotionPhotoVersion": "1", "Container:Directory/Item[2]/Item:Semantic": "MotionPhoto", "Container:Directory/Item[2]/Item:Length": "40"}, {"tk:resource-name": "motion-photo.mp4", "Content-Type": "video/mp4"}]`
 
 			doc, err := tika.Extract(context.TODO(), &provider.ResourceInfo{
 				Type: provider.ResourceType_RESOURCE_TYPE_FILE,
@@ -236,11 +231,6 @@ var _ = Describe("Tika", func() {
 
 		It("drops the motion photo facet when the advertised video is gone", func() {
 			fullResponse = `[{"Camera:MotionPhotoVersion": "1", "Container:Directory/Item[2]/Item:Semantic": "MotionPhoto", "Container:Directory/Item[2]/Item:Length": "40"}]`
-			retriever := &contentMocks.Retriever{}
-			retriever.On("Retrieve", mock.Anything, mock.Anything).Return(io.NopCloser(strings.NewReader("")), nil)
-			retriever.On("RetrieveRange", mock.Anything, mock.Anything, int64(60), mock.Anything).
-				Return(io.NopCloser(strings.NewReader("JFIF leftovers")), nil)
-			tika.Retriever = retriever
 
 			doc, err := tika.Extract(context.TODO(), &provider.ResourceInfo{
 				Type: provider.ResourceType_RESOURCE_TYPE_FILE,