From 376a2e19eab62640257b40db8163a0cab92c7038 Mon Sep 17 00:00:00 2001
From: Earl Warren <contact@earl-warren.org>
Date: Fri, 17 Jan 2025 07:42:20 +0000
Subject: fix: reduce noise for the v303 migration (#6591)

Using SELECT `%s` FROM `%s` WHERE 0 = 1 to assert the existence of a column is simple but noisy: it shows errors in the migrations that are confusing for Forgejo admins because they are not actual errors.

Use introspection instead, which is more complicated but leads to the same result.

Add a test that ensures it works as expected, for all database types. Although the migration is run for all database types, it does not account for various scenarios and is never tested in the case a column does not exist.

Refs: https://codeberg.org/forgejo/forgejo/issues/6583

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6591
Reviewed-by: Otto <otto@codeberg.org>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>
---
 models/migrations/v1_23/v303.go      | 35 +++++++++++++++++++++++-------
 models/migrations/v1_23/v303_test.go | 41 ++++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+), 8 deletions(-)
 create mode 100644 models/migrations/v1_23/v303_test.go

diff --git a/models/migrations/v1_23/v303.go b/models/migrations/v1_23/v303.go
index e3ee180539..2fb37ac843 100644
--- a/models/migrations/v1_23/v303.go
+++ b/models/migrations/v1_23/v303.go
@@ -1,23 +1,27 @@
-// Copyright 2024 The Forgejo Authors.
-// SPDX-License-Identifier: MIT
+// Copyright 2025 The Forgejo Authors.
+// SPDX-License-Identifier: GPL-3.0-or-later
 
 package v1_23 //nolint
 
 import (
-	"fmt"
-
 	"code.gitea.io/gitea/models/migrations/base"
 
 	"xorm.io/xorm"
+	"xorm.io/xorm/schemas"
 )
 
 func GiteaLastDrop(x *xorm.Engine) error {
+	tables, err := x.DBMetas()
+	if err != nil {
+		return err
+	}
+
 	sess := x.NewSession()
 	defer sess.Close()
 
 	for _, drop := range []struct {
-		table string
-		field string
+		table  string
+		column string
 	}{
 		{"badge", "slug"},
 		{"oauth2_application", "skip_secondary_authorization"},
@@ -29,10 +33,25 @@ func GiteaLastDrop(x *xorm.Engine) error {
 		{"protected_branch", "force_push_allowlist_team_i_ds"},
 		{"protected_branch", "force_push_allowlist_deploy_keys"},
 	} {
-		if _, err := sess.Exec(fmt.Sprintf("SELECT `%s` FROM `%s` WHERE 0 = 1", drop.field, drop.table)); err != nil {
+		var table *schemas.Table
+		found := false
+
+		for _, table = range tables {
+			if table.Name == drop.table {
+				found = true
+				break
+			}
+		}
+
+		if !found {
 			continue
 		}
-		if err := base.DropTableColumns(sess, drop.table, drop.field); err != nil {
+
+		if table.GetColumn(drop.column) == nil {
+			continue
+		}
+
+		if err := base.DropTableColumns(sess, drop.table, drop.column); err != nil {
 			return err
 		}
 	}
diff --git a/models/migrations/v1_23/v303_test.go b/models/migrations/v1_23/v303_test.go
new file mode 100644
index 0000000000..752eacee0c
--- /dev/null
+++ b/models/migrations/v1_23/v303_test.go
@@ -0,0 +1,41 @@
+// Copyright 2025 The Forgejo Authors.
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package v1_23 //nolint
+
+import (
+	"testing"
+
+	migration_tests "code.gitea.io/gitea/models/migrations/test"
+
+	"github.com/stretchr/testify/require"
+	"xorm.io/xorm/schemas"
+)
+
+func Test_GiteaLastDrop(t *testing.T) {
+	type Badge struct {
+		ID   int64 `xorm:"pk autoincr"`
+		Slug string
+	}
+
+	x, deferable := migration_tests.PrepareTestEnv(t, 0, new(Badge))
+	defer deferable()
+	if x == nil || t.Failed() {
+		return
+	}
+
+	getColumn := func() *schemas.Column {
+		tables, err := x.DBMetas()
+		require.NoError(t, err)
+		require.Len(t, tables, 1)
+		table := tables[0]
+		require.Equal(t, "badge", table.Name)
+		return table.GetColumn("slug")
+	}
+
+	require.NotNil(t, getColumn(), "slug column exists")
+	require.NoError(t, GiteaLastDrop(x))
+	require.Nil(t, getColumn(), "slug column was deleted")
+	// idempotent
+	require.NoError(t, GiteaLastDrop(x))
+}
-- 
cgit v1.2.3