Initial obs-groups-bot

Makefile

@@ -1,8 +1,4 @@
-MODULES := devel-importer utils/hujson utils/maintainer-update gitea-events-rabbitmq-publisher gitea_status_proxy group-review obs-forward-bot obs-staging-bot obs-status-service workflow-direct workflow-pr
+MODULES := devel-importer utils/hujson utils/maintainer-update gitea-events-rabbitmq-publisher gitea_status_proxy group-review obs-forward-bot obs-groups-bot obs-staging-bot obs-status-service workflow-direct workflow-pr
 
-.PHONY: build $(MODULES)
-
-build: $(MODULES)
-
-$(MODULES):
-	go build -C $@ -buildmode=pie
+build:
+	for m in $(MODULES); do go build -C $$m -buildmode=pie || exit 1 ; done

common/gitea_utils.go

@@ -768,10 +768,6 @@ func (gitea *GiteaTransport) RequestReviews(pr *models.PullRequest, reviewers ..
 		return nil, fmt.Errorf("Cannot create pull request reviews: %w", err)
 	}
 
-	// Invalidate the timeline cache so the next GetTimeline call reflects
-	// the newly created review_requested entry.
-	gitea.ResetTimelineCache(pr.Base.Repo.Owner.UserName, pr.Base.Repo.Name, pr.Index)
-
 	return review.GetPayload(), nil
 }
 
@@ -780,13 +776,6 @@ func (gitea *GiteaTransport) UnrequestReview(org, repo string, id int64, reviwer
 		repository.NewRepoDeletePullReviewRequestsParams().WithOwner(org).WithRepo(repo).WithIndex(id).WithBody(&models.PullReviewRequestOptions{
 			Reviewers: reviwers,
 		}), gitea.transport.DefaultAuthentication)
-
-	if err == nil {
-		// Invalidate the timeline cache so the next GetTimeline call reflects
-		// the newly created review_request_removed entry.
-		gitea.ResetTimelineCache(org, repo, id)
-	}
-
 	return err
 }
 
@@ -872,31 +861,24 @@ func (gitea *GiteaTransport) GetTimeline(org, repo string, idx int64) ([]*models
 	prID := fmt.Sprintf("%s/%s!%d", org, repo, idx)
 	giteaTimelineCacheMutex.RLock()
 	TimelineCache, IsCached := giteaTimelineCache[prID]
-	if IsCached && TimelineCache.lastCheck.Add(time.Second*5).Compare(time.Now()) > 0 {
-		giteaTimelineCacheMutex.RUnlock()
-		return TimelineCache.data, nil
+	var LastCachedTime strfmt.DateTime
+	if IsCached {
+		l := len(TimelineCache.data)
+		if l > 0 {
+			LastCachedTime = TimelineCache.data[0].Updated
+		}
+
+		// cache data for 5 seconds
+		if TimelineCache.lastCheck.Add(time.Second*5).Compare(time.Now()) > 0 {
+			giteaTimelineCacheMutex.RUnlock()
+			return TimelineCache.data, nil
+		}
 	}
 	giteaTimelineCacheMutex.RUnlock()
 
 	giteaTimelineCacheMutex.Lock()
 	defer giteaTimelineCacheMutex.Unlock()
 
-	// Re-read after acquiring the write lock: another goroutine may have
-	// already refreshed the cache while we were waiting.
-	TimelineCache, IsCached = giteaTimelineCache[prID]
-	if IsCached && TimelineCache.lastCheck.Add(time.Second*5).Compare(time.Now()) > 0 {
-		return TimelineCache.data, nil
-	}
-
-	// Find the highest Updated timestamp across all cached items so the
-	// incremental fetch picks up both new entries and modified ones.
-	var LastCachedTime strfmt.DateTime
-	for _, d := range TimelineCache.data {
-		if time.Time(d.Updated).Compare(time.Time(LastCachedTime)) > 0 {
-			LastCachedTime = d.Updated
-		}
-	}
-
 	for resCount > 0 {
 		opts := issue.NewIssueGetCommentsAndTimelineParams().WithOwner(org).WithRepo(repo).WithIndex(idx).WithPage(&page)
 		if !LastCachedTime.IsZero() {

obs-groups-bot/.gitignore

@@ -0,0 +1 @@
+obs-groups-bot

obs-groups-bot/main.go

@@ -0,0 +1,242 @@
+// Connect to the Open Build Service (OBS) API, retrieves a list of all groups,
+// and exports their metadata (specifically member lists) into individual JSON files.
+//
+// The tool supports both command-line flags and environment variables for configuration
+// (not for authentication, which is only via env vars), and includes a debug mode for verbose output.
+// It handles different XML response formats from the OBS API and ensures that
+// the output JSON files are properly sanitized and formatted.
+//
+// The accepted command-line flags are:
+//
+//	-debug: Enable debug output showing API URLs and responses.
+//	-instance: Name of the OBS instance (used in metadata, default "openSUSE").
+//	-host: Base URL of the OBS API (default "http://localhost:3000").
+//	-output: Directory to save the JSON files (default "groups").
+//
+// Usage:
+//
+//		# Using environment variables (OBS_USER, OBS_PASSWORD)
+//		go run main.go
+//
+//		# Targeting a specific OBS instance and output directory
+//		go run main.go -host "https://api.opensuse.org" -output "./obs_groups"
+//
+//	 	# Full command with debug mode
+//	 	go run main.go -host http://localhost:8000 -output "./obs_groups" -instance "OBS" -debug
+
+package main
+
+import (
+	"encoding/json"
+	"encoding/xml"
+	"flag"
+	"fmt"
+	"io"
+	"log"
+	"os"
+	"path/filepath"
+	"strings"
+	"time"
+
+	"src.opensuse.org/autogits/common"
+)
+
+type groupsList struct {
+	XMLName xml.Name    `xml:"groups"`
+	Groups  []groupItem `xml:"group"`
+}
+
+type groupsListAlt struct {
+	XMLName xml.Name     `xml:"directory"`
+	Entries []groupEntry `xml:"entry"`
+}
+
+type groupEntry struct {
+	Name  string `xml:"name,attr,omitempty"`
+	Inner string `xml:",innerxml"`
+}
+
+func (e *groupEntry) getName() string {
+	if e.Name != "" {
+		return e.Name
+	}
+	return e.Inner
+}
+
+type groupItem struct {
+	GroupID string `xml:"groupid,attr"`
+}
+
+func getAllGroups(client *common.ObsClient) ([]string, error) {
+	res, err := client.ObsRequest("GET", []string{"group"}, nil)
+	if err != nil {
+		return nil, err
+	}
+	defer res.Body.Close()
+
+	data, err := io.ReadAll(res.Body)
+	if err != nil {
+		return nil, err
+	}
+
+	log.Printf("Response status: %d, body length: %d", res.StatusCode, len(data))
+	if res.StatusCode != 200 {
+		bodyStr := string(data)
+		if len(bodyStr) > 500 {
+			bodyStr = bodyStr[:500]
+		}
+		return nil, fmt.Errorf("Unexpected return code: %d, body: %s", res.StatusCode, bodyStr)
+	}
+
+	// Try parsing as <groups> format
+	var groupsList groupsList
+	err = xml.Unmarshal(data, &groupsList)
+	if err == nil && len(groupsList.Groups) > 0 {
+		groupIDs := make([]string, len(groupsList.Groups))
+		for i, g := range groupsList.Groups {
+			groupIDs[i] = g.GroupID
+		}
+		return groupIDs, nil
+	}
+
+	// Try parsing as <directory> format
+	var groupsAlt groupsListAlt
+	err = xml.Unmarshal(data, &groupsAlt)
+	if err == nil && len(groupsAlt.Entries) > 0 {
+		groupIDs := make([]string, len(groupsAlt.Entries))
+		for i, e := range groupsAlt.Entries {
+			groupIDs[i] = e.getName()
+		}
+		return groupIDs, nil
+	}
+
+	// Log what we got
+	bodyStr := string(data)
+	if len(bodyStr) > 1000 {
+		bodyStr = bodyStr[:1000]
+	}
+	log.Printf("Failed to parse XML, got: %s", bodyStr)
+	return nil, fmt.Errorf("Could not parse groups response")
+}
+
+type GroupOutput struct {
+	Meta      ImportMeta `json:"_meta,omitempty"`
+	Name      string     `json:"Name"`
+	Reviewers []string   `json:"Reviewers"`
+	Silent    bool       `json:"Silent,omitempty"`
+}
+
+type ImportMeta struct {
+	ImportedFrom string    `json:"imported_from"`
+	ReadOnly     bool      `json:"read_only"`
+	ImportTime   time.Time `json:"import_time"`
+}
+
+func sanitizeFilename(name string) string {
+	name = strings.ReplaceAll(name, "/", "_")
+	name = strings.ReplaceAll(name, ":", "_")
+	name = strings.ReplaceAll(name, " ", "_")
+	return name
+}
+
+func processGroup(client *common.ObsClient, groupID, outputDir, instanceName string, importTime time.Time) error {
+	meta, err := client.GetGroupMeta(groupID)
+	if err != nil {
+		return fmt.Errorf("fetching group meta: %w", err)
+	}
+
+	if meta == nil {
+		return fmt.Errorf("group not found")
+	}
+
+	common.LogDebug(fmt.Sprintf("Group meta for %s: Title: %s, Persons: %d", groupID, meta.Title, len(meta.Persons.Persons)))
+
+	reviewers := make([]string, 0, len(meta.Persons.Persons))
+	for _, p := range meta.Persons.Persons {
+		reviewers = append(reviewers, p.UserID)
+	}
+
+	output := GroupOutput{
+		Meta: ImportMeta{
+			ImportedFrom: instanceName,
+			ReadOnly:     true,
+			ImportTime:   importTime,
+		},
+		Name:      groupID,
+		Reviewers: reviewers,
+	}
+
+	filename := sanitizeFilename(groupID) + ".json"
+	filePath := filepath.Join(outputDir, filename)
+
+	data, err := json.MarshalIndent(output, "", "  ")
+	if err != nil {
+		return fmt.Errorf("marshaling json: %w", err)
+	}
+
+	if err := os.WriteFile(filePath, data, 0644); err != nil {
+		return fmt.Errorf("writing file: %w", err)
+	}
+
+	common.LogDebug(fmt.Sprintf("Saved group %s to %s", groupID, filePath))
+
+	return nil
+}
+
+func main() {
+	debugModePtr := flag.Bool("debug", false, "Enable debug output showing API URLs")
+	obsInstance := flag.String("instance", "openSUSE", "OBS instance name (used in metadata)")
+	obsHost := flag.String("host", "http://localhost:3000", "OBS API host URL")
+	outputDir := flag.String("output", "groups", "Output directory for JSON files")
+	flag.Parse()
+
+	if *debugModePtr {
+		common.SetLoggingLevel(common.LogLevelDebug)
+	}
+
+	if err := common.RequireObsSecretToken(); err != nil {
+		log.Fatal(err)
+	}
+
+	log.Printf("Connecting to OBS at %s (instance: %s)", *obsHost, *obsInstance)
+
+	client, err := common.NewObsClient(*obsHost)
+	if err != nil {
+		log.Fatalf("Failed to create OBS client: %v", err)
+	}
+
+	log.Println("Fetching list of all groups...")
+	groupIDs, err := getAllGroups(client)
+	if err != nil {
+		log.Fatalf("Failed to get groups list: %v", err)
+	}
+
+	log.Printf("Found %d groups: %v", len(groupIDs), groupIDs)
+	log.Printf("Found %s ", groupIDs)
+
+	err = os.MkdirAll(*outputDir, 0755)
+	if err != nil {
+		log.Fatalf("Failed to create output directory: %v", err)
+	}
+
+	importTime := time.Now()
+
+	successCount := 0
+	errorCount := 0
+
+	for i, groupID := range groupIDs {
+		log.Printf("[%d/%d] Fetching group: %s", i+1, len(groupIDs), groupID)
+
+		if err := processGroup(client, groupID, *outputDir, *obsInstance, importTime); err != nil {
+			log.Printf("Error processing group %s: %v", groupID, err)
+			errorCount++
+			continue
+		}
+
+		successCount++
+		time.Sleep(100 * time.Millisecond)
+	}
+
+	log.Printf("Done! Success: %d, Errors: %d", successCount, errorCount)
+	log.Printf("JSON files saved to: %s", *outputDir)
+}

obs-groups-bot/main_test.go

@@ -0,0 +1,211 @@
+package main
+
+import (
+	"encoding/json"
+	"encoding/xml"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"reflect"
+	"testing"
+	"time"
+
+	"src.opensuse.org/autogits/common"
+)
+
+func TestGroupsListParsing(t *testing.T) {
+	// Test <groups> format
+	groupsXML := `<?xml version="1.0" encoding="utf-8"?>
+<groups>
+  <group groupid="group1"/>
+  <group groupid="group2"/>
+  <group groupid="group3"/>
+</groups>`
+
+	var groupsList groupsList
+	err := xml.Unmarshal([]byte(groupsXML), &groupsList)
+	if err != nil {
+		t.Fatalf("Failed to unmarshal groups XML: %v", err)
+	}
+
+	if len(groupsList.Groups) != 3 {
+		t.Errorf("Expected 3 groups, got %d", len(groupsList.Groups))
+	}
+
+	expected := []string{"group1", "group2", "group3"}
+	for i, g := range groupsList.Groups {
+		if g.GroupID != expected[i] {
+			t.Errorf("Expected group %s, got %s", expected[i], g.GroupID)
+		}
+	}
+}
+
+func TestProcessGroup(t *testing.T) {
+	// 1. Mock the OBS API server for GetGroupMeta
+	groupID := "test:group"
+	mockGroupMetaResponse := `<?xml version="1.0" encoding="utf-8"?>
+<group>
+  <title>Test Group Title</title>
+  <person>
+    <person userid="user1" role="maintainer"/>
+    <person userid="user2" role="reviewer"/>
+  </person>
+</group>`
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		expectedPath := "/group/" + groupID
+		if r.URL.Path != expectedPath {
+			t.Errorf("Expected path %s, got %s", expectedPath, r.URL.Path)
+			http.NotFound(w, r)
+			return
+		}
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte(mockGroupMetaResponse))
+	}))
+	defer server.Close()
+
+	// 2. Create a temporary directory for output
+	outputDir := t.TempDir()
+
+	// 3. Initialize client pointing to mock server
+	client, err := common.NewObsClient(server.URL)
+	if err != nil {
+		t.Fatalf("Failed to create client: %v", err)
+	}
+
+	// 4. Call processGroup
+	instanceName := "test-instance"
+	importTime := time.Now().UTC().Truncate(time.Second) // Truncate for stable comparison
+	err = processGroup(client, groupID, outputDir, instanceName, importTime)
+	if err != nil {
+		t.Fatalf("processGroup failed: %v", err)
+	}
+
+	// 5. Verify the output file
+	expectedFilename := sanitizeFilename(groupID) + ".json"
+	filePath := filepath.Join(outputDir, expectedFilename)
+
+	// Check if file exists
+	if _, err := os.Stat(filePath); os.IsNotExist(err) {
+		t.Fatalf("Expected output file was not created: %s", filePath)
+	}
+
+	// Read and verify file content
+	data, err := os.ReadFile(filePath)
+	if err != nil {
+		t.Fatalf("Failed to read output file: %v", err)
+	}
+
+	var result GroupOutput
+	if err := json.Unmarshal(data, &result); err != nil {
+		t.Fatalf("Failed to unmarshal output JSON: %v", err)
+	}
+
+	// Assertions
+	expectedReviewers := []string{"user1", "user2"}
+	expectedOutput := GroupOutput{
+		Meta: ImportMeta{
+			ImportedFrom: instanceName,
+			ReadOnly:     true,
+			ImportTime:   importTime,
+		},
+		Name:      groupID,
+		Reviewers: expectedReviewers,
+	}
+
+	// Use reflect.DeepEqual for a robust comparison of the structs
+	if !reflect.DeepEqual(result, expectedOutput) {
+		t.Errorf("Output JSON does not match expected.\nGot: %+v\nWant: %+v", result, expectedOutput)
+	}
+}
+
+func TestGetAllGroups(t *testing.T) {
+	// Mock the OBS API server
+	mockResponse := `<?xml version="1.0" encoding="utf-8"?>
+<groups>
+  <group groupid="mock-group-1"/>
+  <group groupid="mock-group-2"/>
+</groups>`
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// Verify the request path
+		if r.URL.Path != "/group" {
+			t.Errorf("Expected path /group, got %s", r.URL.Path)
+		}
+		// Verify method
+		if r.Method != "GET" {
+			t.Errorf("Expected method GET, got %s", r.Method)
+		}
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte(mockResponse))
+	}))
+	defer server.Close()
+
+	// Initialize client pointing to mock server
+	client, err := common.NewObsClient(server.URL)
+	if err != nil {
+		t.Fatalf("Failed to create client: %v", err)
+	}
+
+	groups, err := getAllGroups(client)
+	if err != nil {
+		t.Fatalf("GetAllGroups failed: %v", err)
+	}
+
+	if len(groups) != 2 {
+		t.Errorf("Expected 2 groups, got %d", len(groups))
+	}
+	if groups[0] != "mock-group-1" {
+		t.Errorf("Expected first group to be mock-group-1, got %s", groups[0])
+	}
+}
+
+func TestGroupsListDirectoryFormat(t *testing.T) {
+	// Test <directory> format with name attribute
+	dirXML := `<?xml version="1.0" encoding="utf-8"?>
+<directory>
+  <entry name="group-a"/>
+  <entry name="group-b"/>
+  <entry name="group-c"/>
+</directory>`
+
+	var groupsAlt groupsListAlt
+	err := xml.Unmarshal([]byte(dirXML), &groupsAlt)
+	if err != nil {
+		t.Fatalf("Failed to unmarshal directory XML: %v", err)
+	}
+
+	if len(groupsAlt.Entries) != 3 {
+		t.Errorf("Expected 3 entries, got %d", len(groupsAlt.Entries))
+	}
+
+	expected := []string{"group-a", "group-b", "group-c"}
+	for i, e := range groupsAlt.Entries {
+		if e.getName() != expected[i] {
+			t.Errorf("Expected entry %s, got %s", expected[i], e.getName())
+		}
+	}
+}
+
+func TestSanitizeFilename(t *testing.T) {
+	tests := []struct {
+		input    string
+		expected string
+	}{
+		{"simple", "simple"},
+		{"group/name", "group_name"},
+		{"project:group", "project_group"},
+		{"group with spaces", "group_with_spaces"},
+		{"group/name:space", "group_name_space"},
+		{"", ""},
+		{"multiple///slashes", "multiple___slashes"},
+	}
+
+	for _, tc := range tests {
+		result := sanitizeFilename(tc.input)
+		if result != tc.expected {
+			t.Errorf("sanitizeFilename(%q) = %q, expected %q", tc.input, result, tc.expected)
+		}
+	}
+}

workflow-pr/pr_processor.go

@@ -8,7 +8,6 @@ import (
 	"runtime/debug"
 	"slices"
 	"strings"
-	"sync"
 	"time"
 
 	"github.com/opentracing/opentracing-go/log"
@@ -629,29 +628,9 @@ func (pr *PRProcessor) Process(req *models.PullRequest) error {
 	return err
 }
 
-// prLocks serialises concurrent processing of the same PR.
-// Both the RabbitMQ event loop and the consistency-checker goroutine call
-// ProcesPullRequest; without this lock they can race on reviewer add/remove.
-// Key format: "org/repo#num"
-var prLocks sync.Map // map[string]chan struct{}
-
-func prLockKey(pr *models.PullRequest) string {
-	return fmt.Sprintf("%s/%s#%d", pr.Base.Repo.Owner.UserName, pr.Base.Repo.Name, pr.Index)
-}
-
-func acquirePRLock(key string) chan struct{} {
-	v, _ := prLocks.LoadOrStore(key, make(chan struct{}, 1))
-	ch := v.(chan struct{})
-	ch <- struct{}{}
-	return ch
-}
-
-func releasePRLock(ch chan struct{}) {
-	<-ch
-}
-
 type RequestProcessor struct {
 	configuredRepos map[string][]*common.AutogitConfig
+	recursive       int
 }
 
 func (w *RequestProcessor) Process(pr *models.PullRequest) error {
@@ -668,9 +647,6 @@ func ProcesPullRequest(pr *models.PullRequest, configs []*common.AutogitConfig)
 		return nil
 	}
 
-	lock := acquirePRLock(prLockKey(pr))
-	defer releasePRLock(lock)
-
 	PRProcessor, err := AllocatePRProcessor(pr, configs)
 	if err != nil {
 		log.Error(err)
@@ -687,23 +663,17 @@ func (w *RequestProcessor) ProcessFunc(request *common.Request) (err error) {
 			common.LogInfo("panic cought --- recovered")
 			common.LogError(string(debug.Stack()))
 		}
+		w.recursive--
 	}()
 
+	w.recursive++
+	if w.recursive > 3 {
+		common.LogError("Recursion limit reached... something is wrong with this PR?")
+		return nil
+	}
+
 	var pr *models.PullRequest
 	if req, ok := request.Data.(*common.PullRequestWebhookEvent); ok {
-		// Skip pull_request_sync events triggered by the bot's own pushes to
-		// prjgit branches.  Those would re-run AssignReviewers immediately
-		// after the bot itself just set them, producing spurious add/remove
-		// cycles.  Human-triggered syncs have a different sender and are still
-		// processed normally.
-		if request.Type == common.RequestType_PRSync && CurrentUser != nil &&
-			req.Sender.Username == CurrentUser.UserName {
-			common.LogDebug("Skipping self-triggered pull_request_sync from", req.Sender.Username,
-				"on", req.Pull_Request.Base.Repo.Owner.Username+"/"+req.Pull_Request.Base.Repo.Name,
-				"#", req.Pull_Request.Number)
-			return nil
-		}
-
 		pr, err = Gitea.GetPullRequest(req.Pull_Request.Base.Repo.Owner.Username, req.Pull_Request.Base.Repo.Name, req.Pull_Request.Number)
 		if err != nil {
 			common.LogError("Cannot find PR for issue:", req.Pull_Request.Base.Repo.Owner.Username, req.Pull_Request.Base.Repo.Name, req.Pull_Request.Number)
@@ -740,16 +710,8 @@ func (w *RequestProcessor) ProcessFunc(request *common.Request) (err error) {
 		common.LogError("*** Cannot find config for org:", pr.Base.Repo.Owner.UserName)
 	}
 	if err = ProcesPullRequest(pr, configs); err == updatePrjGitError_requeue {
-		// Retry after a delay in a background goroutine so the event loop is
-		// not blocked while we wait.  The per-PR lock inside ProcesPullRequest
-		// ensures no other processing races with the retry.
-		go func() {
-			time.Sleep(time.Second * 5)
-			if err := ProcesPullRequest(pr, configs); err != nil {
-				common.LogError("requeue retry failed:", err)
-			}
-		}()
-		return nil
+		time.Sleep(time.Second * 5)
+		return w.ProcessFunc(request)
 	}
 	return err
 }

workflow-pr/pr_processor_test.go

@@ -989,6 +989,18 @@ func TestProcessFunc(t *testing.T) {
 		}
 	})
 
+	t.Run("Recursion limit", func(t *testing.T) {
+		reqProc.recursive = 3
+		err := reqProc.ProcessFunc(&common.Request{})
+		if err != nil {
+			t.Errorf("Expected nil error on recursion limit, got %v", err)
+		}
+		if reqProc.recursive != 3 {
+			t.Errorf("Expected recursive to be 3, got %d", reqProc.recursive)
+		}
+		reqProc.recursive = 0 // Reset
+	})
+
 	t.Run("Invalid data format", func(t *testing.T) {
 		err := reqProc.ProcessFunc(&common.Request{Data: nil})
 		if err == nil || !strings.Contains(err.Error(), "Invalid data format") {