Make SetterMethods ContainerUpdaterMethods

This commit separated SetterMethods that are specifically for updating
containers to another method type, named ContainerUpdaterMethod. This
change is done to force a context parameter into container setters,
allowing the frontend to know if an incoming update is valid or not,
based on the state of the context given.

The validity check should be the same as any other context:

    select {
    case <-ctx.Done():
        return
    default:
        addEvent()
    }

It is crucial, however, to do the checking and updating in the same
thread or lock as the context is cancelled. This explicit
synchronization is required to prevent any race condition whatsoever
with cancellation of the context.

The backend must pass in the right context, that is, any context that
inherits the cancellation from the frontend. Passing in the invalid
context is undefined behavior and will eventually cause a data race.
This commit is contained in:
diamondburned 2021-05-01 21:41:39 -07:00
parent 410ac73469
commit 8bfabf58ec
5 changed files with 51 additions and 35 deletions

View File

@ -355,7 +355,7 @@ type Identifier interface {
// Labels given to the frontend may contain images or avatars, and the frontend
// has the choice to display them or not.
type LabelContainer interface {
SetLabel(text.Rich)
SetLabel(context.Context, text.Rich)
}
// ListMember represents a single member in the member list. Note that this
@ -452,7 +452,7 @@ type MemberDynamicSection interface {
type MemberListContainer interface {
// RemoveMember removes a member from a section. If neither the member nor the
// section exists, then the client should ignore it.
RemoveMember(sectionID ID, memberID ID)
RemoveMember(ctx context.Context, sectionID ID, memberID ID)
// SetMember adds or updates (or upsert) a member into a section. This operation
// must not change the section's member count. As such, changes should be done
// separately in SetSection. If the section does not exist, then the client
@ -462,12 +462,12 @@ type MemberListContainer interface {
// Typically, the backend should try and avoid calling this method and instead
// update the labeler in the name. This method should only be used for adding
// members.
SetMember(sectionID ID, member ListMember)
SetMember(ctx context.Context, sectionID ID, member ListMember)
// SetSections (re)sets the list of sections to be the given slice. Members from
// the old section list should be transferred over to the new section entry if
// the section name's content is the same. Old sections that don't appear in the
// new slice should be removed.
SetSections(sections []MemberSection)
SetSections(ctx context.Context, sections []MemberSection)
}
// MemberLister adds a member list into a message server.
@ -537,12 +537,12 @@ type MessageUpdate interface {
// allowed to have multiple views. This is usually done with tabs or splits, but
// the backend should update them all nonetheless.
type MessagesContainer interface {
DeleteMessage(MessageDelete)
UpdateMessage(MessageUpdate)
DeleteMessage(context.Context, MessageDelete)
UpdateMessage(context.Context, MessageUpdate)
// CreateMessage inserts a message into the container. The frontend must
// guarantee that the messages are in order based on what's returned from
// Time().
CreateMessage(MessageCreate)
CreateMessage(context.Context, MessageCreate)
}
// Messenger is for servers that contain messages. This is similar to Discord or
@ -618,10 +618,10 @@ type ReadContainer interface {
// their read indicators. The backend can use this to free up users/authors that
// are no longer in the server, for example when they are offline or have left
// the server.
DeleteIndications(authorIDs []ID)
DeleteIndications(ctx context.Context, authorIDs []ID)
// AddIndications adds a map of users/authors to the respective message ID of
// the server that implements ReadIndicator.
AddIndications([]ReadIndication)
AddIndications(context.Context, []ReadIndication)
}
// ReadIndicator adds a read indicator API for frontends to show. An example of
@ -721,7 +721,7 @@ type ServerUpdate interface {
// as servers can be infinitely nested. Frontends should also reset the entire
// node and its children when SetServers is called again.
type ServersContainer interface {
UpdateServer(ServerUpdate)
UpdateServer(context.Context, ServerUpdate)
// SetServer is called by the backend service to request a reset of the server
// list. The frontend can choose to call Servers() on each of the given servers,
// or it can call that later. The backend should handle both cases.
@ -731,7 +731,7 @@ type ServersContainer interface {
// should only be considered empty if it's an empty non-nil slice. An
// unavailable list, on the other hand, can be treated as backend issues, e.g. a
// connection issue.
SetServers([]Server)
SetServers(context.Context, []Server)
}
// Service is a complete service that's capable of multiple sessions. It has to
@ -835,11 +835,11 @@ type TypingContainer interface {
// of typers. This function is usually not needed, as the client will take care
// of removing them after TypingTimeout has been reached or other conditions
// listed in ServerMessageTypingIndicator are met.
RemoveTyper(authorID ID)
RemoveTyper(ctx context.Context, authorID ID)
// AddTyper appends the typer (author) into the frontend's list of typers, or it
// pushes this typer on top of others. The frontend should assume current time
// every time AddTyper is called.
AddTyper(User)
AddTyper(context.Context, User)
}
// TypingIndicator optionally extends ServerMessage to provide bidirectional
@ -885,7 +885,7 @@ type TypingIndicator interface {
type UnreadContainer interface {
// SetUnread sets the container's unread state to the given boolean. The
// frontend may choose how to represent this.
SetUnread(unread bool, mentioned bool)
SetUnread(ctx context.Context, unread bool, mentioned bool)
}
// UnreadIndicator adds an unread state API for frontends to use. The unread
@ -908,7 +908,7 @@ type UnreadIndicator interface {
// the frontend has no use in knowing the error. As such, marking messages as
// read is best-effort. The backend is in charge of synchronizing the read state
// with the server and coordinating it with reasonable rate limits, if needed.
MarkRead(messageID ID)
MarkRead(ctx context.Context, messageID ID)
}
// User is the interface for an identifiable author. The interface defines that

View File

@ -67,6 +67,9 @@ func generateInterfaces(ifaces []repository.Interface) jen.Code {
case repository.SetterMethod:
stmt.Params(generateFuncParams(method.Parameters, "")...)
stmt.Params(generateFuncParamsErr(repository.NamedType{}, method.ErrorType)...)
case repository.ContainerUpdaterMethod:
stmt.Params(generateFuncParamsCtx(method.Parameters, "")...)
stmt.Params(generateFuncParamsErr(repository.NamedType{}, method.ErrorType)...)
case repository.IOMethod:
stmt.Params(generateFuncParamsCtx(method.Parameters, "")...)
stmt.Params(generateFuncParamsErr(method.ReturnValue, method.ErrorType)...)

Binary file not shown.

View File

@ -10,6 +10,7 @@ func init() {
gob.Register(AsserterMethod{})
gob.Register(GetterMethod{})
gob.Register(SetterMethod{})
gob.Register(ContainerUpdaterMethod{})
gob.Register(IOMethod{})
}
@ -67,11 +68,7 @@ func (m GetterMethod) ReturnError() bool {
}
// SetterMethod is a method that sets values. These methods must not do IO, and
// they have to be non-blocking. They're used only for containers. Actual setter
// methods implemented by the backend belongs to IOMethods.
//
// Clients should always keep track of the values given to setters and free them
// once the setters are called again with new values.
// they have to be non-blocking.
type SetterMethod struct {
method
@ -84,6 +81,22 @@ type SetterMethod struct {
ErrorType string
}
// ContainerUpdaterMethod is a SetterMethod that passes to the container the
// current context to prevent race conditions when synchronizing.
// The rule of thumb is that any setter method done inside a method with a
// context is usually this type of method.
type ContainerUpdaterMethod struct {
method
// Parameters is the list of parameters in the function. These parameters
// should be the parameters to set.
Parameters []NamedType
// ErrorType is non-empty if the function returns an error at the end of
// returns. An error may be returned from the backend if the input is
// invalid, but it must not do IO. Frontend setters must never error.
ErrorType string
}
// IOMethod is a regular method that can do IO and thus is blocking. These
// methods usually always return errors. IOMethods must always have means of
// cancelling them in the API, but implementations don't have to use it; as

View File

@ -1307,7 +1307,7 @@ var Main = Packages{
`},
Name: "UnreadIndicator",
Methods: []Method{
SetterMethod{
ContainerUpdaterMethod{
method: method{
Comment: Comment{`
MarkRead marks a message in the server messenger as
@ -1446,7 +1446,7 @@ var Main = Packages{
`},
Name: "ServersContainer",
Methods: []Method{
SetterMethod{
ContainerUpdaterMethod{
method: method{
Comment: Comment{`
SetServer is called by the backend service to
@ -1467,7 +1467,7 @@ var Main = Packages{
},
Parameters: []NamedType{{Type: "[]Server"}},
},
SetterMethod{
ContainerUpdaterMethod{
method: method{Name: "UpdateServer"},
Parameters: []NamedType{{Type: "ServerUpdate"}},
},
@ -1527,7 +1527,7 @@ var Main = Packages{
`},
Name: "MessagesContainer",
Methods: []Method{
SetterMethod{
ContainerUpdaterMethod{
method: method{
Comment: Comment{`
CreateMessage inserts a message into the container.
@ -1538,11 +1538,11 @@ var Main = Packages{
},
Parameters: []NamedType{{Type: "MessageCreate"}},
},
SetterMethod{
ContainerUpdaterMethod{
method: method{Name: "UpdateMessage"},
Parameters: []NamedType{{Type: "MessageUpdate"}},
},
SetterMethod{
ContainerUpdaterMethod{
method: method{Name: "DeleteMessage"},
Parameters: []NamedType{{Type: "MessageDelete"}},
},
@ -1631,7 +1631,7 @@ var Main = Packages{
`},
Name: "LabelContainer",
Methods: []Method{
SetterMethod{
ContainerUpdaterMethod{
method: method{Name: "SetLabel"},
Parameters: []NamedType{{
Type: MakeQual("text", "Rich"),
@ -1647,7 +1647,7 @@ var Main = Packages{
`},
Name: "ReadContainer",
Methods: []Method{
SetterMethod{
ContainerUpdaterMethod{
method: method{
Comment: Comment{`
AddIndications adds a map of users/authors to the
@ -1658,7 +1658,7 @@ var Main = Packages{
},
Parameters: []NamedType{{"", "[]ReadIndication"}},
},
SetterMethod{
ContainerUpdaterMethod{
method: method{
Comment: Comment{`
DeleteIndications deletes a list of unused
@ -1692,7 +1692,7 @@ var Main = Packages{
`},
Name: "UnreadContainer",
Methods: []Method{
SetterMethod{
ContainerUpdaterMethod{
method: method{
Comment: Comment{`
SetUnread sets the container's unread state to the
@ -1718,7 +1718,7 @@ var Main = Packages{
`},
Name: "TypingContainer",
Methods: []Method{
SetterMethod{
ContainerUpdaterMethod{
method: method{
Comment: Comment{`
AddTyper appends the typer (author) into the
@ -1730,7 +1730,7 @@ var Main = Packages{
},
Parameters: []NamedType{{"", "User"}},
},
SetterMethod{
ContainerUpdaterMethod{
method: method{
Comment: Comment{`
RemoveTyper explicitly removes the typer with the
@ -1770,7 +1770,7 @@ var Main = Packages{
`},
Name: "MemberListContainer",
Methods: []Method{
SetterMethod{
ContainerUpdaterMethod{
method: method{
Comment: Comment{`
SetSections (re)sets the list of sections to be the
@ -1786,7 +1786,7 @@ var Main = Packages{
{Name: "sections", Type: "[]MemberSection"},
},
},
SetterMethod{
ContainerUpdaterMethod{
method: method{
Comment: Comment{`
SetMember adds or updates (or upsert) a member into
@ -1809,7 +1809,7 @@ var Main = Packages{
{"member", "ListMember"},
},
},
SetterMethod{
ContainerUpdaterMethod{
method: method{
Comment: Comment{`
RemoveMember removes a member from a section. If