From 5d8af336a081776d37eba556adbf278e679b09c3 Mon Sep 17 00:00:00 2001
From: Justin Hiemstra <jhiemstra@wisc.edu>
Date: Mon, 14 Oct 2024 19:00:02 +0000
Subject: [PATCH] Switch to use new Caps struct but still handle old JSON from
 Director

This change fixes the stuff I broke in commit ba186d2 by handling previously-
unconsidered backwards compatibility issues. Now, unmarshalling the NSAdV2
struct will correctly populate capabilities even if the source JSON uses
the old style of capabilities.

This also fixes the capabilities displayed for each origin at the Director.
---
 server_structs/director.go      | 67 ++++++++++++++++++++++++++++++---
 server_structs/director_test.go | 47 +++++++++++++++++++++++
 2 files changed, 108 insertions(+), 6 deletions(-)

diff --git a/server_structs/director.go b/server_structs/director.go
index 316df7d43..d65ddd1d5 100644
--- a/server_structs/director.go
+++ b/server_structs/director.go
@@ -45,13 +45,21 @@ type (
 		CredentialIssuer url.URL      `json:"issuer"`
 	}
 
-	// Note that the json are kept in uppercase for backward compatibility
+	// A struct for unmarshalling the old JSON while we stage the breaking change across releases
+	OldCapabilities struct {
+		PublicRead   bool
+		Read         bool
+		Write        bool
+		Listing      bool
+		FallBackRead bool
+	}
+
 	Capabilities struct {
-		PublicReads bool `json:"PublicRead"`
-		Reads       bool `json:"Read"`
-		Writes      bool `json:"Write"`
-		Listings    bool `json:"Listing"`
-		DirectReads bool `json:"FallBackRead"`
+		PublicReads bool
+		Reads       bool
+		Writes      bool
+		Listings    bool
+		DirectReads bool
 	}
 
 	NamespaceAdV2 struct {
@@ -179,6 +187,53 @@ type (
 	}
 )
 
+// A helper function to handle JSON->NSAdV2 unmarshalling across multiple deprecated JSON keys.
+//
+// When the Director sends a list of NamespaceAdV2 structs as JSON, it may contain
+// the old capabilities struct. This function checks if the raw JSON contains the
+// "FallbackRead" field, indicating the old capabilities struct, and unmarshals
+// the JSON into the new struct accordingly. This is to ensure backwards compatibility
+// We can probably think about removing this function when we don't see origins/directors
+// running Pelican <= v7.11.0.
+func (n *NamespaceAdV2) UnmarshalJSON(data []byte) error {
+	// Use alias struct of NamespaceAdV2 to prevent infinite recursion of UnmarshalJSON
+	type Alias NamespaceAdV2
+	aux := &struct {
+		Caps json.RawMessage `json:"Caps"`
+		*Alias
+	}{
+		Alias: (*Alias)(n),
+	}
+
+	if err := json.Unmarshal(data, &aux); err != nil {
+		return err
+	}
+
+	// Check if the raw Caps JSON contains "FallbackRead", indicating
+	// the old caps struct. Since the struct-to-JSON code has never included
+	// 'omitempty', we can safely assume that the field is ALWAYS present in
+	// old versions of the JSON.
+	if strings.Contains(string(aux.Caps), "FallBackRead") {
+		var oldCaps OldCapabilities
+		if err := json.Unmarshal(aux.Caps, &oldCaps); err != nil {
+			return err
+		}
+
+		// Map old capabilities to new capabilities
+		n.Caps.PublicReads = oldCaps.PublicRead
+		n.Caps.Reads = oldCaps.Read
+		n.Caps.Writes = oldCaps.Write
+		n.Caps.Listings = oldCaps.Listing
+		n.Caps.DirectReads = oldCaps.FallBackRead
+	} else {
+		if err := json.Unmarshal(aux.Caps, &n.Caps); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 func (x XPelNs) GetName() string {
 	return "X-Pelican-Namespace"
 }
diff --git a/server_structs/director_test.go b/server_structs/director_test.go
index c05e1b1f1..3364866bf 100644
--- a/server_structs/director_test.go
+++ b/server_structs/director_test.go
@@ -19,6 +19,7 @@
 package server_structs
 
 import (
+	"encoding/json"
 	"fmt"
 	"net/http"
 	"net/url"
@@ -281,3 +282,49 @@ func TestXPelTokGenParsing(t *testing.T) {
 		assert.Len(t, xPelTokGen.BasePaths, 0)
 	})
 }
+
+func TestNSAdV2UnmarshalJSON(t *testing.T) {
+	oldJSON := `{"PublicRead":true,"Caps":{"PublicRead":true,"Read":true,"Write":false,"Listing":false,"FallBackRead":true},"path":"/ncar","token-generation":[{"strategy":"","vault-server":"","max-scope-depth":0,"issuer":{"Scheme":"","Opaque":"","User":null,"Host":"","Path":"","RawPath":"","OmitHost":false,"ForceQuery":false,"RawQuery":"","Fragment":"","RawFragment":""}}],"token-issuer":[],"from-topology":true}`
+	newJSON := `{"Caps":{"PublicReads":false,"Reads":true,"Writes":false,"Listings":false,"DirectReads":true},"path":"/ncar","token-generation":[{"strategy":"","vault-server":"","max-scope-depth":0,"issuer":{"Scheme":"","Opaque":"","User":null,"Host":"","Path":"","RawPath":"","OmitHost":false,"ForceQuery":false,"RawQuery":"","Fragment":"","RawFragment":""}}],"token-issuer":[],"from-topology":true}`
+
+	tests := []struct {
+		name     string
+		jsonData string
+		expected Capabilities
+	}{
+		{
+			name:     "Old JSON format",
+			jsonData: oldJSON,
+			expected: Capabilities{
+				PublicReads: true,
+				Reads:       true,
+				Writes:      false,
+				Listings:    false,
+				DirectReads: true,
+			},
+		},
+		{
+			name:     "New JSON format",
+			jsonData: newJSON,
+			expected: Capabilities{
+				PublicReads: false,
+				Reads:       true,
+				Writes:      false,
+				Listings:    false,
+				DirectReads: true,
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var ns NamespaceAdV2
+			if err := json.Unmarshal([]byte(tt.jsonData), &ns); err != nil {
+				t.Fatalf("UnmarshalJSON() error = %v", err)
+			}
+			if ns.Caps != tt.expected {
+				t.Errorf("UnmarshalJSON() = %v, want %v", ns.Caps, tt.expected)
+			}
+		})
+	}
+}