refactor(services): avoid specific service methods shadow their impl. from the service framework

Author: renatomaia

internal/advancer/advancer_test.go

@@ -48,7 +48,7 @@ func newMockAdvancerServiceWithBatchSize(
 		repository:     repo,
 	}
 	serviceArgs := &service.CreateInfo{Name: "advancer", Impl: s, EnableReschedule: true}
-	err := service.Create(context.Background(), serviceArgs, &s.Service)
+	err := service.NewTickService(serviceArgs, &s.TickService)
 	if err != nil {
 		return nil, err
 	}
@@ -97,12 +97,12 @@ func (s *AdvancerSuite) TestServiceInterface() {
 		repository.GetEpochsReturn = map[common.Address][]*Epoch{
 			machineManager.Map[1].application.IApplicationAddress: {},
 		}
-		tickErrors := advancer.Tick()
+		tickErrors := advancer.Tick(context.Background())
 		require.Empty(tickErrors)
 
 		// Test Tick with error
 		repository.GetEpochsError = errors.New("list epochs error")
-		tickErrors = advancer.Tick()
+		tickErrors = advancer.Tick(context.Background())
 		require.NotEmpty(tickErrors)
 		require.Contains(tickErrors[0].Error(), "list epochs error")
 
@@ -1547,7 +1547,7 @@ func (s *AdvancerSuite) TestSelfWakeOnSuccess() {
 	require.NoError(err)
 
 	// Call Tick() which internally calls Step() and signals reschedule.
-	svc.Tick()
+	svc.Tick(context.Background())
 
 	// The reschedule channel should have a pending signal.
 	require.True(svc.DrainReschedule(),
@@ -1571,7 +1571,7 @@ func (s *AdvancerSuite) TestNoSelfWakeWhenIdle() {
 	svc, err := newMockAdvancerService(mm, repo)
 	require.NoError(err)
 
-	svc.Tick()
+	svc.Tick(context.Background())
 
 	require.False(svc.DrainReschedule(),
 		"reschedule channel should be empty when no work exists")
@@ -1590,7 +1590,7 @@ func (s *AdvancerSuite) TestNoSelfWakeOnError() {
 	svc, err := newMockAdvancerService(mm, repo)
 	require.NoError(err)
 
-	errs := svc.Tick()
+	errs := svc.Tick(context.Background())
 	require.NotEmpty(errs)
 
 	require.False(svc.DrainReschedule(),
@@ -1631,7 +1631,7 @@ func (s *AdvancerSuite) TestPartialSuccessStillReschedules() {
 
 	// Call Tick — app1 fails, app2 succeeds with more work remaining (batch limit hit).
 	// Tick should surface the error AND signal reschedule for app2's pending work.
-	errs := svc.Tick()
+	errs := svc.Tick(context.Background())
 	require.NotEmpty(errs, "Tick should surface app1's error")
 
 	// Reschedule SHOULD fire: app2 had work, and one failing app must not

internal/advancer/service.go

@@ -24,7 +24,7 @@ const httpShutdownTimeout = 10 * time.Second //nolint: mnd
 
 // Service is the main advancer service that processes inputs through Cartesi machines
 type Service struct {
-	service.Service
+	service.TickService
 	inputBatchSize uint64
 	snapshotsDir   string
 	repository     AdvancerRepository
@@ -54,10 +54,11 @@ func Create(ctx context.Context, c *CreateInfo) (service.IService, error) {
 	}
 
 	s := &Service{}
+	s.TickImpl = s
 	c.Impl = s
 	c.EnableReschedule = true
 
-	err = service.Create(ctx, &c.CreateInfo, &s.Service)
+	err = service.NewTickService(&c.CreateInfo, &s.TickService)
 	if err != nil {
 		return nil, err
 	}
@@ -96,15 +97,12 @@ func Create(ctx context.Context, c *CreateInfo) (service.IService, error) {
 
 	s.LogConfig(c.Config)
 
-	return &s.Service, nil
+	return s, nil
 }
 
 // Service interface implementation
-func (s *Service) Alive() bool     { return true }
-func (s *Service) Ready() bool     { return true }
-func (s *Service) Reload() []error { return nil }
-func (s *Service) Tick() []error {
-	hadWork, err := s.Step(s.Context)
+func (s *Service) Tick(ctx context.Context) []error {
+	hadWork, err := s.Step(ctx)
 
 	// Signal reschedule whenever work was done, even if some apps errored.
 	// Failed apps are marked Failed and removed by the machine manager,
@@ -119,25 +117,14 @@ func (s *Service) Tick() []error {
 	}
 	// During shutdown, the machine manager is closed and GetMachine() may
 	// return ErrNoApp. Suppress this to avoid spurious ERR log entries.
-	if errors.Is(err, ErrNoApp) && s.IsStopping() {
+	if errors.Is(err, ErrNoApp) && ctx.Err() != nil {
 		s.Logger.Warn("Tick interrupted by shutdown", "error", err)
 		return nil
 	}
 	return []error{err}
 }
 
-func (s *Service) Stop(b bool) []error {
-	// CAS achieves once-semantics: the second caller returns immediately
-	// (fire-and-forget) rather than blocking like sync.Once. This is safe
-	// because the orchestrator calls Cancel() after Stop() and waits for
-	// the Serve goroutine to exit.
-	if !s.cleanedUp.CompareAndSwap(false, true) {
-		return nil // already stopped
-	}
-	// This method shadows service.Service.Stop(), so set the stopping flag
-	// explicitly. Without this, a concurrent Tick that observes closed
-	// resources would not see IsStopping() == true.
-	s.SetStopping()
+func (s *Service) fonStop(b bool) []error {
 	var errs []error
 	if s.HTTPServer != nil {
 		s.Logger.Info("Shutting down inspect HTTP server")
@@ -155,17 +142,15 @@ func (s *Service) Stop(b bool) []error {
 	}
 	return errs
 }
-func (s *Service) Serve() error {
+
+func (s *Service) OnServe(ctx context.Context) error {
 	if s.inspector != nil && s.HTTPServerFunc != nil {
 		go func() {
 			if err := s.HTTPServerFunc(); err != nil && !errors.Is(err, http.ErrServerClosed) {
 				s.Logger.Error("Inspect HTTP server failed — shutting down", "error", err)
-				s.Cancel()
+				s.Stop(true)
 			}
 		}()
 	}
-	return s.Service.Serve()
-}
-func (s *Service) String() string {
-	return s.Name
+	return s.TickService.OnServe(ctx)
 }

internal/claimer/claimer_test.go

@@ -188,8 +188,10 @@ func newServiceMock() (*Service, *claimerRepositoryMock, *claimerBlockchainMock)
 	blockchain := &claimerBlockchainMock{}
 
 	claimer := &Service{
-		Service: service.Service{
-			Logger: slog.New(handler),
+		TickService: service.TickService{
+			Service: service.Service{
+				Logger: slog.New(handler),
+			},
 		},
 		submissionEnabled: true,
 		claimsInFlight:    map[int64]common.Hash{},

internal/claimer/service.go

@@ -30,7 +30,7 @@ type CreateInfo struct {
 }
 
 type Service struct {
-	service.Service
+	service.TickService
 
 	repository iclaimerRepository
 	blockchain iclaimerBlockchain
@@ -117,37 +117,20 @@ func Create(ctx context.Context, c *CreateInfo) (service.IService, error) {
 	return &s.Service, nil
 }
 
-func (s *Service) Alive() bool {
-	return true
-}
-
-func (s *Service) Ready() bool {
-	return true
-}
-
-func (s *Service) Reload() []error {
-	return nil
-}
-
-func (s *Service) Stop(bool) []error {
-	s.SetStopping()
-	return nil
-}
-
 // NOTE: tick is not re-entrant!
-func (s *Service) Tick() []error {
+func (s *Service) Tick(ctx context.Context) []error {
 	errs := []error{}
 
 	// gather epochs pairs with open claims, either:
 	// - computed but not yet submitted
-	acceptedOrSubmittedEpochs, computedEpochs, computedApps, errSubmitted := s.repository.SelectSubmittedClaimPairsPerApp(s.Context)
+	acceptedOrSubmittedEpochs, computedEpochs, computedApps, errSubmitted := s.repository.SelectSubmittedClaimPairsPerApp(ctx)
 	if errSubmitted != nil {
 		errs = append(errs, errSubmitted)
 		return errs
 	}
 
 	// - submitted but not yet accepted.
-	acceptedEpochs, submittedEpochs, submittedApps, errAccepted := s.repository.SelectAcceptedClaimPairsPerApp(s.Context)
+	acceptedEpochs, submittedEpochs, submittedApps, errAccepted := s.repository.SelectAcceptedClaimPairsPerApp(ctx)
 	if errAccepted != nil {
 		errs = append(errs, errAccepted)
 		return errs
@@ -164,7 +147,7 @@ func (s *Service) Tick() []error {
 	}
 
 	// we have claims to check. Get the latest/safe/finalized, etc. block
-	defaultBlockNumber, err := s.blockchain.getDefaultBlockNumber(s.Context)
+	defaultBlockNumber, err := s.blockchain.getDefaultBlockNumber(ctx)
 	if err != nil {
 		errs = append(errs, err)
 		return errs

internal/evmreader/service.go

@@ -140,43 +140,20 @@ func (s *Service) Ready() bool {
 	return s.ready.Load()
 }
 
-func (s *Service) Reload() []error {
-	return nil
-}
-
-func (s *Service) Stop(bool) []error {
-	s.SetStopping()
-	return nil
-}
-
-func (s *Service) Tick() []error {
-	return []error{}
-}
-
-func (s *Service) Serve() error {
+func (s *Service) OnServe(ctx context.Context) error {
 	s.alive.Store(true)
-	ready := make(chan struct{}, 1)
-	go func() {
-		defer s.alive.Store(false)
-		defer s.ready.Store(false)
-		err := s.Run(s.Context, ready)
-		if err != nil && s.Context.Err() == nil {
-			s.Logger.Error("Run exited with error", "error", err)
-		}
-		s.Cancel()
-	}()
+	var ready chan struct{}
 	go func() {
 		select {
 		case <-ready:
 			s.ready.Store(true)
-		case <-s.Context.Done():
+		case <-ctx.Done():
 		}
 	}()
-	return s.Service.Serve()
-}
 
-func (s *Service) String() string {
-	return s.Name
+	defer s.alive.Store(false)
+	defer s.ready.Store(false)
+	return s.Run(ctx, ready)
 }
 
 func (s *Service) setupPersistentConfig(

internal/jsonrpc/service.go

@@ -92,25 +92,7 @@ func Create(ctx context.Context, c *CreateInfo) (service.IService, error) {
 	return &s.Service, nil
 }
 
-func (s *Service) Alive() bool {
-	return true
-}
-
-func (s *Service) Ready() bool {
-	return true
-}
-
-func (s *Service) Reload() []error {
-	return nil
-}
-
-func (s *Service) Tick() []error {
-	// No periodic tasks.
-	return nil
-}
-
-func (s *Service) Stop(_ bool) []error {
-	s.SetStopping()
+func (s *Service) OnStop(_ bool) []error {
 	var errs []error
 	s.Logger.Info("Shutting down JSON-RPC HTTP server", "addr", s.server.Addr)
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) //nolint: mnd
@@ -121,11 +103,7 @@ func (s *Service) Stop(_ bool) []error {
 	return errs
 }
 
-func (s *Service) String() string {
-	return s.Name
-}
-
-func (s *Service) Serve() error {
+func (s *Service) OnServe(ctx context.Context) error {
 	listener, err := s.listen("tcp", s.server.Addr)
 	if err != nil {
 		return err

internal/node/node.go

@@ -142,18 +142,15 @@ func (me *Service) Ready() bool {
 	return allReady
 }
 
-func (s *Service) Reload() []error { return nil }
-func (s *Service) Tick() []error   { return nil }
-func (me *Service) Stop(force bool) []error {
-	me.SetStopping()
+func (me *Service) OnStop(force bool) []error {
 	errs := []error{}
 	for _, s := range me.Children {
 		errs = append(errs, s.Stop(force)...)
 	}
 	return errs
 }
 
-func (me *Service) Serve() error {
+func (me *Service) OnServe(ctx context.Context) error {
 	for _, s := range me.Children {
 		go s.Serve()
 	}

internal/prt/service.go

@@ -29,7 +29,7 @@ type CreateInfo struct {
 }
 
 type Service struct {
-	service.Service
+	service.TickService
 	repository        prtRepository
 	client            EthClientInterface
 	adapterFactory    AdapterFactory
@@ -123,10 +123,6 @@ func Create(ctx context.Context, c *CreateInfo) (service.IService, error) {
 	return &s.Service, nil
 }
 
-func (s *Service) Alive() bool     { return true }
-func (s *Service) Ready() bool     { return true }
-func (s *Service) Reload() []error { return nil }
-
 // Tick executes the Validator main logic of producing claims and/or proofs
 // for processed epochs of all running applications.
 func (s *Service) Tick() []error {