Removed BaseModel and all duplicated fields

Author: glpatcern

pkg/share/manager/sql/conversions.go

@@ -111,7 +111,7 @@ func convertToCS3OCMShare(s *model.OcmShare, am []*ocm.AccessMethod) *ocm.Share
 	}
 	if s.Expiration.Valid {
 		share.Expiration = &types.Timestamp{
-			Seconds: uint64(s.Expiration.Int64),
+			Seconds: uint64(s.Expiration.V.Unix()),
 		}
 	}
 	return share
@@ -149,7 +149,7 @@ func convertToCS3OCMReceivedShare(s *model.OcmReceivedShare, p []*ocm.Protocol)
 	}
 	if s.Expiration.Valid {
 		share.Expiration = &types.Timestamp{
-			Seconds: uint64(s.Expiration.Int64),
+			Seconds: uint64(s.Expiration.V.Unix()),
 		}
 	}
 	return share

pkg/share/manager/sql/model/model.go

@@ -19,7 +19,6 @@
 package model
 
 import (
-	"database/sql"
 	"strconv"
 	"time"
 
@@ -29,7 +28,7 @@ import (
 	resourcespb "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1"
 	link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1"
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
-	typespb "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
+	types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
 	conversions "github.com/cs3org/reva/v3/pkg/cbox/utils"
 
 	"gorm.io/datatypes"
@@ -95,37 +94,31 @@ type ShareID struct {
 	ID uint `gorm:"primarykey"`
 }
 
-// This is the base model for all share types. We cannot use gorm.Model, because we want our ID
-// to be a foreign key to ShareID, but we incorporate the date fields from gorm. This struct
-// does not declare any unique indexes as it is embedded by others.
-type BaseModel struct {
+// This is the base model for all share types and embeds parts of gorm.Model. We can't use it, because
+// we want our ID to be a foreign key to ShareID, but we incorporate the date fields from GORM.
+// The commented-out fields would logically belong here, but we define them in each specific type
+// to control the unique indexes to be enforced in the corresponding tables.
+type ProtoShare struct {
 	// Id has to be called Id and not ID, otherwise the foreign key will not work
 	// ID is a special field in GORM, which it uses as the default Primary Key
 	Id        uint    `gorm:"primaryKey;not null;autoIncrement:false"`
 	ShareId   ShareID `gorm:"foreignKey:Id;references:ID;constraint:OnDelete:CASCADE"` //;references:ID
 	CreatedAt time.Time
 	UpdatedAt time.Time
-	DeletedAt gorm.DeletedAt `gorm:"index"`
-}
-
-// ProtoShare contains fields that are common between PublicLinks and Shares.
-// We also define some indexes for performance reasons.
-type ProtoShare struct {
-	BaseModel
+	//DeletedAt  gorm.DeletedAt `gorm:"index"`
+	//Inode      string `gorm:"size:32;index"`
+	//Instance   string `gorm:"size:32;index"`
 	UIDOwner     string   `gorm:"size:64"`
 	UIDInitiator string   `gorm:"size:64;index"`
 	ItemType     ItemType `gorm:"size:16;index"` // file | folder | reference | symlink
 	InitialPath  string
-	Inode        string `gorm:"size:32;index"`
-	Instance     string `gorm:"size:32;index"`
 	Permissions  uint8
 	Orphan       bool
 	Expiration   datatypes.NullTime `gorm:"index"`
 }
 
 // Share is a regular share between users or groups. The unique index ensures that there
-// can only be one share per (inode, instance, permissions, recipient) tuple, unless the share is deleted:
-// for that, we redeclare `DeletedAt`, `Inode`, and `Instance` for GORM to define the unique index.
+// can only be one share per (inode, instance, permissions, recipient) tuple, unless the share is deleted.
 type Share struct {
 	ProtoShare
 	DeletedAt         gorm.DeletedAt `gorm:"uniqueIndex:u_share"`
@@ -139,7 +132,10 @@ type Share struct {
 // PublicLink is a public link share. We only enforce a unique constraint on the token.
 type PublicLink struct {
 	ProtoShare
-	Token                        string `gorm:"uniqueIndex:u_link_token;size:32"` // Current tokens are only 16 chars long, but old tokens used to be 32 characters
+	DeletedAt                    gorm.DeletedAt `gorm:"index"`
+	Inode                        string         `gorm:"size:32"`
+	Instance                     string         `gorm:"size:32"`
+	Token                        string         `gorm:"uniqueIndex:u_link_token;size:32"` // Current tokens are only 16 chars long, but old tokens used to be 32 characters
 	Quicklink                    bool
 	NotifyUploads                bool
 	NotifyUploadsExtraRecipients string
@@ -159,21 +155,21 @@ type ShareState struct {
 }
 
 // OcmShare represents an OCM share for a remote user. The unique index ensures that there
-// can only be one share per (inode, instance, shareWith, owner) tuple, unless the share is deleted:
-// for that, we redeclare `DeletedAt`, `Inode`, and `Instance` as in Share. In addition, tokens must be unique.
+// can only be one share per (inode, instance, recipient) tuple, unless the share is deleted.
+// In addition, tokens must be unique.
+// TODO(lopresti) see if we can consolidate Owner and Initiator with UIDOwner and UIDInitiator in ProtoShare
 type OcmShare struct {
-	BaseModel
+	ProtoShare
 	DeletedAt     gorm.DeletedAt     `gorm:"uniqueIndex:u_ocmshare"`
 	Inode         string             `gorm:"size:64;not null;uniqueIndex:u_ocmshare"`
 	Instance      string             `gorm:"size:64;not null;uniqueIndex:u_ocmshare"`
 	Token         string             `gorm:"size:255;not null;uniqueIndex:u_ocmshare_token"`
 	Name          string             `gorm:"type:text;not null"`
 	ShareWith     string             `gorm:"size:255;not null;uniqueIndex:u_ocmshare"`
-	Owner         string             `gorm:"size:255;not null;uniqueIndex:u_ocmshare"`
+	Owner         string             `gorm:"size:255;not null"`
 	Initiator     string             `gorm:"type:text;not null"`
 	Ctime         uint64             `gorm:"not null"`
 	Mtime         uint64             `gorm:"not null"`
-	Expiration    sql.NullInt64      `gorm:"default:null"`
 	RecipientType OcmShareType       `gorm:"not null"`
 	Protocols     []OcmShareProtocol `gorm:"constraint:OnDelete:CASCADE;"`
 }
@@ -189,18 +185,18 @@ type OcmShareProtocol struct {
 // OcmReceivedShare represents an OCM share received from a remote user.
 type OcmReceivedShare struct {
 	gorm.Model
-	RemoteShareID string        `gorm:"index:i_ocmrecshare_remoteshareid;not null"`
-	Name          string        `gorm:"size:255;not null"`
-	ItemType      ItemType      `gorm:"size:16;not null"`
-	ShareWith     string        `gorm:"size:255;not null"`
-	Owner         string        `gorm:"index:i_ocmrecshare_owner;size:255;not null"`
-	Initiator     string        `gorm:"index:i_ocmrecshare_initiator;size:255;not null"`
-	Ctime         uint64        `gorm:"not null"`
-	Mtime         uint64        `gorm:"not null"`
-	Expiration    sql.NullInt64 `gorm:"default:null"`
-	RecipientType OcmShareType  `gorm:"index:i_ocmrecshare_type;not null"`
-	State         OcmShareState `gorm:"index:i_ocmrecshare_state;not null"`
-	Alias         string        `gorm:"size:64"`
+	RemoteShareID string             `gorm:"index:i_ocmrecshare_remoteshareid;not null"`
+	Name          string             `gorm:"size:255;not null"`
+	ItemType      ItemType           `gorm:"size:16;not null"`
+	ShareWith     string             `gorm:"size:255;not null"`
+	Owner         string             `gorm:"index:i_ocmrecshare_owner;size:255;not null"`
+	Initiator     string             `gorm:"index:i_ocmrecshare_initiator;size:255;not null"`
+	Ctime         uint64             `gorm:"not null"`
+	Mtime         uint64             `gorm:"not null"`
+	Expiration    datatypes.NullTime `gorm:"index"`
+	RecipientType OcmShareType       `gorm:"index:i_ocmrecshare_type;not null"`
+	State         OcmShareState      `gorm:"index:i_ocmrecshare_state;not null"`
+	Alias         string             `gorm:"size:64"`
 	Hidden        bool
 }
 
@@ -219,10 +215,10 @@ type OcmReceivedShareProtocol struct {
 }
 
 func (s *Share) AsCS3Share(granteeType userpb.UserType) *collaboration.Share {
-	creationTs := &typespb.Timestamp{
+	creationTs := &types.Timestamp{
 		Seconds: uint64(s.CreatedAt.Unix()),
 	}
-	updateTs := &typespb.Timestamp{
+	updateTs := &types.Timestamp{
 		Seconds: uint64(s.UpdatedAt.Unix()),
 	}
 	share := &collaboration.Share{
@@ -244,7 +240,7 @@ func (s *Share) AsCS3Share(granteeType userpb.UserType) *collaboration.Share {
 	}
 
 	if s.Expiration.Valid {
-		share.Expiration = &typespb.Timestamp{
+		share.Expiration = &types.Timestamp{
 			Seconds: uint64(s.Expiration.V.Unix()),
 		}
 	}
@@ -271,16 +267,16 @@ func (s *Share) AsCS3ReceivedShare(state *ShareState, granteeType userpb.UserTyp
 }
 
 func (p *PublicLink) AsCS3PublicShare() *link.PublicShare {
-	ts := &typespb.Timestamp{
+	ts := &types.Timestamp{
 		Seconds: uint64(p.CreatedAt.Unix()),
 	}
 
-	var expires *typespb.Timestamp
+	var expires *types.Timestamp
 	if p.Expiration.Valid {
 		exp, err := p.Expiration.Value()
 		if err == nil {
 			expiration := exp.(time.Time)
-			expires = &typespb.Timestamp{
+			expires = &types.Timestamp{
 				Seconds: uint64(expiration.Unix()),
 			}
 		}

pkg/share/manager/sql/ocm_shares.go

@@ -38,6 +38,7 @@ import (
 	"github.com/pkg/errors"
 	"google.golang.org/genproto/protobuf/field_mask"
 	"google.golang.org/protobuf/proto"
+	"gorm.io/datatypes"
 	"gorm.io/gorm"
 
 	_ "github.com/go-sql-driver/mysql"
@@ -101,8 +102,10 @@ func (m *mgr) StoreShare(ctx context.Context, s *ocm.Share) (*ocm.Share, error)
 			RecipientType: convertFromCS3OCMShareType(s.ShareType),
 		}
 		if s.Expiration != nil {
-			share.Expiration.Int64 = int64(s.Expiration.Seconds)
-			share.Expiration.Valid = true
+			share.Expiration = datatypes.NullTime{
+				V:     time.Unix(int64(s.Expiration.Seconds), 0),
+				Valid: true,
+			}
 		}
 		share.Id = id
 		share.ShareId = model.ShareID{ID: id}
@@ -274,10 +277,11 @@ func (m *mgr) StoreReceivedShare(ctx context.Context, s *ocm.ReceivedShare) (*oc
 			RecipientType: convertFromCS3OCMShareType(s.ShareType),
 			State:         convertFromCS3OCMShareState(s.State),
 		}
-
 		if s.Expiration != nil {
-			receivedShare.Expiration.Int64 = int64(s.Expiration.Seconds)
-			receivedShare.Expiration.Valid = true
+			receivedShare.Expiration = datatypes.NullTime{
+				V:     time.Unix(int64(s.Expiration.Seconds), 0),
+				Valid: true,
+			}
 		}
 
 		id := tx.Create(receivedShare)
@@ -480,7 +484,7 @@ func (m *mgr) getByID(ctx context.Context, user *userpb.User, id *ocm.ShareId) (
 func (m *mgr) getByKey(ctx context.Context, user *userpb.User, key *ocm.ShareKey) (*ocm.Share, error) {
 	var shareModel model.OcmShare
 	if err := m.db.WithContext(ctx).
-		Where("owner = ? AND storage_id = ? AND file_id = ? AND share_with = ? AND (initiator = ? OR owner = ?)",
+		Where("owner = ? AND instance = ? AND inode = ? AND share_with = ? AND (initiator = ? OR owner = ?)",
 			key.Owner.OpaqueId, key.ResourceId.StorageId, key.ResourceId.OpaqueId, formatUserID(key.Grantee.GetUserId()), user.Id.OpaqueId, user.Id.OpaqueId).
 		First(&shareModel).Error; err != nil {
 		if errors.Is(err, gorm.ErrRecordNotFound) {
@@ -562,7 +566,7 @@ func (m *mgr) deleteByID(ctx context.Context, user *userpb.User, id *ocm.ShareId
 
 func (m *mgr) deleteByKey(ctx context.Context, user *userpb.User, key *ocm.ShareKey) error {
 	result := m.db.WithContext(ctx).
-		Where("owner = ? AND storage_id = ? AND file_id = ? AND share_with = ? AND (initiator = ? OR owner = ?)",
+		Where("owner = ? AND instance = ? AND inode = ? AND share_with = ? AND (initiator = ? OR owner = ?)",
 			key.Owner.OpaqueId, key.ResourceId.StorageId, key.ResourceId.OpaqueId, formatUserID(key.Grantee.GetUserId()), user.Id.OpaqueId, user.Id.OpaqueId).
 		Delete(&model.OcmShare{})
 
@@ -675,7 +679,7 @@ func translateFilters(filters []*ocm.ListOCMSharesRequest_Filter) (string, []any
 		for n, f := range lst {
 			switch filter := f.Term.(type) {
 			case *ocm.ListOCMSharesRequest_Filter_ResourceId:
-				filterQuery.WriteString("storage_id = ? AND file_id = ?")
+				filterQuery.WriteString("instance = ? AND inode = ?")
 				params = append(params, filter.ResourceId.StorageId, filter.ResourceId.OpaqueId)
 			case *ocm.ListOCMSharesRequest_Filter_Creator:
 				filterQuery.WriteString("initiator = ?")

pkg/share/manager/sql/public_link.go

@@ -109,11 +109,8 @@ func (m *PublicShareMgr) CreatePublicShare(ctx context.Context, u *user.User, md
 		return nil, errors.Wrap(err, "failed to create id for PublicShare")
 	}
 
-	publiclink.BaseModel = model.BaseModel{
-		Id:      id,
-		ShareId: model.ShareID{ID: id},
-	}
-
+	publiclink.Id = id
+	publiclink.ShareId = model.ShareID{ID: id}
 	publiclink.UIDOwner = conversions.FormatUserID(md.Owner)
 	publiclink.UIDInitiator = conversions.FormatUserID(user.Id)
 	publiclink.InitialPath = md.Path
@@ -512,9 +509,7 @@ func emptyLinkWithId(id string) (*model.PublicLink, error) {
 	}
 	link := &model.PublicLink{
 		ProtoShare: model.ProtoShare{
-			BaseModel: model.BaseModel{
-				Id: uint(intId),
-			},
+			Id: uint(intId),
 		},
 	}
 	return link, nil

pkg/share/manager/sql/share.go

@@ -122,11 +122,8 @@ func (m *ShareMgr) Share(ctx context.Context, md *provider.ResourceInfo, g *coll
 		return nil, errors.Wrap(err, "failed to create id for PublicShare")
 	}
 
-	share.BaseModel = model.BaseModel{
-		Id:      id,
-		ShareId: model.ShareID{ID: id},
-	}
-
+	share.Id = id
+	share.ShareId = model.ShareID{ID: id}
 	share.UIDOwner = conversions.FormatUserID(md.Owner)
 	share.UIDInitiator = conversions.FormatUserID(user.Id)
 	share.InitialPath = md.Path
@@ -606,9 +603,7 @@ func emptyShareWithId(id string) (*model.Share, error) {
 	}
 	share := &model.Share{
 		ProtoShare: model.ProtoShare{
-			BaseModel: model.BaseModel{
-				Id: uint(intId),
-			},
+			Id: uint(intId),
 		},
 	}
 	return share, nil