Vehicle: refresh drivesomethinggreater on portal schedule

[andig]

The EU Data Act portal stores a new dataset roughly every 15 minutes and answers the datadelivery list endpoint with 404 "No files available for this request" until the vehicle has delivered its first dataset.

• Map that 404 to api.ErrNotAvailable so readings report "not available" instead of a raw status error.
• Status now returns the dataset timestamp, parsed from the compact file-name prefix (e.g. 20260531102941_VIN_..._no_content_found.zip), with createdOn as fallback.
• Log the data points

Refs #30324

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• The Provider's use of timer *time.Timer in NewProvider looks racy under concurrent calls, since v.timer is read/written and timer.Stop()/time.AfterFunc are invoked without any synchronization; consider guarding timer with a mutex or otherwise ensuring thread safety around cache reset scheduling.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The `Provider`'s use of `timer *time.Timer` in `NewProvider` looks racy under concurrent calls, since `v.timer` is read/written and `timer.Stop()`/`time.AfterFunc` are invoked without any synchronization; consider guarding `timer` with a mutex or otherwise ensuring thread safety around cache reset scheduling.

## Individual Comments

### Comment 1
<location path="vehicle/vw/eudataact/provider.go" line_range="29" />
<code_context>
+// margin), so the data is refreshed as soon as the portal delivers it.
 type Provider struct {
 	statusG func() (map[string]string, error)
+	timer   *time.Timer
 }

</code_context>
<issue_to_address>
**issue (bug_risk):** Timer field updates are not synchronized and can lead to data races under concurrent access

The supplier passed to `util.ResettableCached` may run in multiple goroutines. Inside that supplier you read, stop, and reassign `v.timer` without synchronization, causing a data race on both the field and the underlying `time.Timer`, which is not concurrency-safe.

Please guard all accesses and `Stop` calls on `v.timer` with a mutex (e.g., wrap it in a small struct with its own lock). Alternatively, avoid storing the timer on `Provider` and keep it internal to the cache implementation if that better suits the design.
</issue_to_address>

### Comment 2
<location path="vehicle/vw/eudataact/provider.go" line_range="37" />
<code_context>
+	v := &Provider{}
+
+	var cached util.Cacheable[map[string]string]
+	cached = util.ResettableCached(func() (map[string]string, error) {
+		data, ts, err := api.Status(vin)
+		if err == nil && !ts.IsZero() {
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the timer management into a Provider helper method to simplify the cache closure and clarify responsibilities.

You can simplify the lifecycle concerns a bit without changing behaviour by:

1. Pulling the timer logic out of the cache factory into a small helper method on `Provider`.
2. Making the self-referential use of `cached.Reset` more explicit/isolated.

That keeps the API and semantics intact, but reduces the amount of work happening inside the closure and makes the responsibilities clearer.

For example:

```go
type Provider struct {
	statusG func() (map[string]string, error)
	timer   *time.Timer
}

func (p *Provider) scheduleReset(ts time.Time, reset func()) {
	if ts.IsZero() {
		return
	}

	if p.timer != nil {
		p.timer.Stop()
	}

	p.timer = time.AfterFunc(resetDelay(ts, time.Now()), reset)
}

func NewProvider(api *API, vin string, cache time.Duration) *Provider {
	v := &Provider{}

	var cached util.Cacheable[map[string]string]
	cached = util.ResettableCached(func() (map[string]string, error) {
		data, ts, err := api.Status(vin)
		if err == nil {
			// delegate timer management to Provider helper
			v.scheduleReset(ts, cached.Reset)
		}
		return data, err
	}, cache)

	v.statusG = cached.Get
	return v
}
```

This keeps:

- `util.ResettableCached` usage unchanged.
- The self-referential pattern (`cached.Reset` in the closure) intact, so behaviour is the same.

But it:

- Removes timer stop/restart details from the cache factory, so the closure only coordinates fetching + scheduling.
- Centralises timer lifecycle into `scheduleReset`, making it easier to reason about and unit test independently.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}