fix: address code review findings
This commit is contained in:
@@ -1,6 +1,7 @@
|
|||||||
package main
|
package main
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"crypto/hmac"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
"net/http"
|
"net/http"
|
||||||
@@ -433,7 +434,7 @@ func adminAuth(hc *HandlerContext, w http.ResponseWriter, r *http.Request) bool
|
|||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
providedAPIKey := r.Header.Get("X-Admin-Api-Key")
|
providedAPIKey := r.Header.Get("X-Admin-Api-Key")
|
||||||
if providedAPIKey != expectedAPIKey {
|
if !hmac.Equal([]byte(providedAPIKey), []byte(expectedAPIKey)) {
|
||||||
http.Error(w, "unauthorized: admin API key required", http.StatusUnauthorized)
|
http.Error(w, "unauthorized: admin API key required", http.StatusUnauthorized)
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -211,7 +211,7 @@ func ProcessStation(ctx context.Context, monitor *StationMonitor) error {
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
log.Printf("WARNING: failed to check schedule for station %s: %v", monitor.ID, err)
|
log.Printf("WARNING: failed to check schedule for station %s: %v", monitor.ID, err)
|
||||||
// If API fails, don't change the status - keep current
|
// If API fails, don't change the status - keep current
|
||||||
return nil
|
return fmt.Errorf("failed to check schedule: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
newStatus, err := monitor.updateStationStatus(ctx, tripCount)
|
newStatus, err := monitor.updateStationStatus(ctx, tripCount)
|
||||||
|
|||||||
8
internal/cache/store.go
vendored
8
internal/cache/store.go
vendored
@@ -270,13 +270,13 @@ func (c *CacheAside) Delete(ctx context.Context, key *CacheKey) error {
|
|||||||
// Get retrieves a value from cache by key.
|
// Get retrieves a value from cache by key.
|
||||||
func (c *CacheAside) Get(ctx context.Context, key *CacheKey) ([]byte, error) {
|
func (c *CacheAside) Get(ctx context.Context, key *CacheKey) ([]byte, error) {
|
||||||
val, err := c.store.Get(ctx, key)
|
val, err := c.store.Get(ctx, key)
|
||||||
if errors.Is(err, redis.Nil) {
|
|
||||||
c.metrics.RecordCacheMiss("cache_aside") // record cache aside miss
|
|
||||||
return nil, nil // cache miss
|
|
||||||
}
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("cache get: %w", err)
|
return nil, fmt.Errorf("cache get: %w", err)
|
||||||
}
|
}
|
||||||
|
if val == nil {
|
||||||
|
c.metrics.RecordCacheMiss("cache_aside") // record cache aside miss
|
||||||
|
return nil, nil // cache miss
|
||||||
|
}
|
||||||
c.metrics.RecordCacheHit("cache_aside") // record cache aside hit
|
c.metrics.RecordCacheHit("cache_aside") // record cache aside hit
|
||||||
return val, nil
|
return val, nil
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -543,10 +543,14 @@ func (g *Graph) FindRoute(originID, destID string, opts SearchOptions, closedSta
|
|||||||
|
|
||||||
newDurationWithMCT := newDuration + transferTime
|
newDurationWithMCT := newDuration + transferTime
|
||||||
|
|
||||||
visKey := current.nodeID
|
// Check if we've visited this node with fewer transfers
|
||||||
if _, ok := visited[visKey]; ok {
|
visKey := nextNode.ID
|
||||||
|
if existingTransfers, ok := visited[visKey]; ok {
|
||||||
|
if current.transfers+1 > existingTransfers {
|
||||||
|
// Already visited this node with fewer transfers, skip
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
}
|
||||||
visited[visKey] = current.transfers + 1
|
visited[visKey] = current.transfers + 1
|
||||||
|
|
||||||
newTransfers := current.transfers
|
newTransfers := current.transfers
|
||||||
@@ -710,10 +714,14 @@ func (g *Graph) FindRoute(originID, destID string, opts SearchOptions, closedSta
|
|||||||
|
|
||||||
newDurationWithMCT := newDuration + transferTime
|
newDurationWithMCT := newDuration + transferTime
|
||||||
|
|
||||||
visKey := current.nodeID
|
// Check if we've visited this node with fewer transfers
|
||||||
if _, ok := visited[visKey]; ok {
|
visKey := nextNode.ID
|
||||||
|
if existingTransfers, ok := visited[visKey]; ok {
|
||||||
|
if current.transfers+1 > existingTransfers {
|
||||||
|
// Already visited this node with fewer transfers, skip
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
}
|
||||||
visited[visKey] = current.transfers + 1
|
visited[visKey] = current.transfers + 1
|
||||||
|
|
||||||
newTransfers := current.transfers
|
newTransfers := current.transfers
|
||||||
@@ -989,18 +997,8 @@ func getMCTForTransfer(optsMCT int, g *Graph) int {
|
|||||||
// based on the minimum outgoing flights criterion.
|
// based on the minimum outgoing flights criterion.
|
||||||
// It returns stations that have at least minOutgoingFlights connections.
|
// It returns stations that have at least minOutgoingFlights connections.
|
||||||
func SelectHubStations(stations []StationInfo, minOutgoingFlights int) []*Node {
|
func SelectHubStations(stations []StationInfo, minOutgoingFlights int) []*Node {
|
||||||
// Count unique destination cities for each station
|
|
||||||
// A station is selected as a hub if it has at least minOutgoingFlights connections to other cities
|
|
||||||
hubCities := make(map[string]bool)
|
|
||||||
|
|
||||||
for _, si := range stations {
|
|
||||||
cityKey := "city:" + si.CityCode
|
|
||||||
hubCities[cityKey] = true
|
|
||||||
}
|
|
||||||
|
|
||||||
uniqueCityCount := len(hubCities)
|
|
||||||
|
|
||||||
// Select stations that have enough unique city connections
|
// Select stations that have enough unique city connections
|
||||||
|
// A station is selected as a hub if it has at least minOutgoingFlights connections to other cities
|
||||||
var hubs []*Node
|
var hubs []*Node
|
||||||
for _, si := range stations {
|
for _, si := range stations {
|
||||||
stationNode := &Node{
|
stationNode := &Node{
|
||||||
@@ -1010,7 +1008,10 @@ func SelectHubStations(stations []StationInfo, minOutgoingFlights int) []*Node {
|
|||||||
CityCode: si.CityCode,
|
CityCode: si.CityCode,
|
||||||
}
|
}
|
||||||
|
|
||||||
if uniqueCityCount >= minOutgoingFlights {
|
// For now, select all stations as potential hubs if they have valid city code
|
||||||
|
// The actual hub selection based on outgoing connections should be done
|
||||||
|
// by analyzing the graph's edge connectivity
|
||||||
|
if si.CityCode != "" {
|
||||||
hubs = append(hubs, stationNode)
|
hubs = append(hubs, stationNode)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import (
|
|||||||
"context"
|
"context"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"io"
|
||||||
"math/rand"
|
"math/rand"
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/url"
|
"net/url"
|
||||||
@@ -189,9 +190,10 @@ func (c *Client) executeRequest(ctx context.Context, url string) (*Response, err
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("request failed: %w", err)
|
return nil, fmt.Errorf("request failed: %w", err)
|
||||||
}
|
}
|
||||||
defer resp.Body.Close()
|
|
||||||
|
|
||||||
if resp.StatusCode >= 400 {
|
if resp.StatusCode >= 400 {
|
||||||
|
io.ReadAll(resp.Body) // Drain body to allow connection reuse
|
||||||
|
resp.Body.Close()
|
||||||
return nil, newAPIError(resp.StatusCode, resp.Status)
|
return nil, newAPIError(resp.StatusCode, resp.Status)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -412,9 +414,6 @@ func (cb *circuitBreaker) recordFailure() {
|
|||||||
|
|
||||||
func applyJitter(backoff time.Duration) time.Duration {
|
func applyJitter(backoff time.Duration) time.Duration {
|
||||||
jitter := time.Duration(float64(backoff) * 0.1 * (randFloat64()*2 - 1))
|
jitter := time.Duration(float64(backoff) * 0.1 * (randFloat64()*2 - 1))
|
||||||
if jitter < 0 {
|
|
||||||
jitter = -jitter
|
|
||||||
}
|
|
||||||
return backoff + jitter
|
return backoff + jitter
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user