Skip to content

Commit

Permalink
Merge pull request PelicanPlatform#924 from haoming29/consolidate-ns-…
Browse files Browse the repository at this point in the history
…exist

Consolidate two `namespaceExists` DB function
  • Loading branch information
jhiemstrawisc authored Mar 25, 2024
2 parents 1833c7c + 4f3a0b8 commit f8ac1da
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 44 deletions.
6 changes: 3 additions & 3 deletions registry/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ func keySignChallengeCommit(ctx *gin.Context, data *registrationData) (bool, map
log.Debug("Registering namespace ", data.Prefix)

// Check if prefix exists before doing anything else
exists, err := namespaceExists(data.Prefix)
exists, err := namespaceExistsByPrefix(data.Prefix)
if err != nil {
log.Errorf("Failed to check if namespace already exists: %v", err)
return false, nil, errors.Wrap(err, "Server encountered an error checking if namespace already exists")
Expand Down Expand Up @@ -605,7 +605,7 @@ func deleteNamespaceHandler(ctx *gin.Context) {
}

// Check if prefix exists before trying to delete it
exists, err := namespaceExists(prefix)
exists, err := namespaceExistsByPrefix(prefix)
if err != nil {
ctx.JSON(http.StatusInternalServerError, gin.H{"error": "server encountered an error checking if namespace already exists"})
log.Errorf("Failed to check if the namespace already exists: %v", err)
Expand Down Expand Up @@ -737,7 +737,7 @@ func wildcardHandler(ctx *gin.Context) {
} else if strings.HasSuffix(path, "/.well-known/openid-configuration") {
// Check that the namespace exists before constructing config JSON
prefix := strings.TrimSuffix(path, "/.well-known/openid-configuration")
exists, err := namespaceExists(prefix)
exists, err := namespaceExistsByPrefix(prefix)
if err != nil {
ctx.JSON(http.StatusInternalServerError, gin.H{"error": "Server encountered an error while checking if the prefix exists"})
log.Errorf("Error while checking for existence of prefix %s: %v", prefix, err)
Expand Down
13 changes: 1 addition & 12 deletions registry/registry_db.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ func createTopologyTable() error {
}

// Check if a namespace exists in either Topology or Pelican registry
func namespaceExists(prefix string) (bool, error) {
func namespaceExistsByPrefix(prefix string) (bool, error) {
var count int64

err := db.Model(&Namespace{}).Where("prefix = ?", prefix).Count(&count).Error
Expand Down Expand Up @@ -244,17 +244,6 @@ func namespaceExistsById(id int) (bool, error) {
}
}

func namespaceExistsByPrefix(prefix string) (bool, error) {
var namespaces []Namespace
result := db.Where("prefix = ?", prefix).Limit(1).Find(&namespaces)

if result.Error != nil {
return false, result.Error
} else {
return result.RowsAffected > 0, nil
}
}

func namespaceBelongsToUserId(id int, userId string) (bool, error) {
var result Namespace
err := db.First(&result, "id = ?", id).Error
Expand Down
36 changes: 8 additions & 28 deletions registry/registry_db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,26 +159,6 @@ var (
}
)

func TestNamespaceExistsByPrefix(t *testing.T) {
setupMockRegistryDB(t)
defer teardownMockNamespaceDB(t)

t.Run("return-false-for-prefix-dne", func(t *testing.T) {
found, err := namespaceExistsByPrefix("/non-existed-namespace")
require.NoError(t, err)
assert.False(t, found)
})

t.Run("return-true-for-existing-ns", func(t *testing.T) {
resetNamespaceDB(t)
err := insertMockDBData([]Namespace{{Prefix: "/foo"}})
require.NoError(t, err)
found, err := namespaceExistsByPrefix("/foo")
require.NoError(t, err)
assert.True(t, found)
})
}

func TestGetNamespaceById(t *testing.T) {
setupMockRegistryDB(t)
defer teardownMockNamespaceDB(t)
Expand Down Expand Up @@ -785,12 +765,12 @@ func TestRegistryTopology(t *testing.T) {
require.NoError(t, err)

// Check that topology namespace exists
exists, err := namespaceExists("/topo/foo")
exists, err := namespaceExistsByPrefix("/topo/foo")
require.NoError(t, err)
require.True(t, exists)

// Check that topology namespace exists
exists, err = namespaceExists("/topo/bar")
exists, err = namespaceExistsByPrefix("/topo/bar")
require.NoError(t, err)
require.True(t, exists)

Expand All @@ -806,12 +786,12 @@ func TestRegistryTopology(t *testing.T) {
require.NoError(t, err)

// Check that the regular namespace exists
exists, err = namespaceExists("/regular/foo")
exists, err = namespaceExistsByPrefix("/regular/foo")
require.NoError(t, err)
require.True(t, exists)

// Check that a bad namespace doesn't exist
exists, err = namespaceExists("/bad/namespace")
exists, err = namespaceExistsByPrefix("/bad/namespace")
require.NoError(t, err)
require.False(t, exists)

Expand All @@ -830,22 +810,22 @@ func TestRegistryTopology(t *testing.T) {
require.NoError(t, err)

// Check that /topo/foo still exists
exists, err = namespaceExists("/topo/foo")
exists, err = namespaceExistsByPrefix("/topo/foo")
require.NoError(t, err)
require.True(t, exists)

// And that /topo/baz was added
exists, err = namespaceExists("/topo/baz")
exists, err = namespaceExistsByPrefix("/topo/baz")
require.NoError(t, err)
require.True(t, exists)

// Check that /topo/bar is gone
exists, err = namespaceExists("/topo/bar")
exists, err = namespaceExistsByPrefix("/topo/bar")
require.NoError(t, err)
require.False(t, exists)

// Finally, check that /regular/foo survived
exists, err = namespaceExists("/regular/foo")
exists, err = namespaceExistsByPrefix("/regular/foo")
require.NoError(t, err)
require.True(t, exists)

Expand Down
2 changes: 1 addition & 1 deletion registry/registry_ui.go
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ func createUpdateNamespace(ctx *gin.Context, isUpdate bool) {

if !isUpdate {
// Check if prefix exists before doing anything else. Skip check if it's update operation
exists, err := namespaceExists(ns.Prefix)
exists, err := namespaceExistsByPrefix(ns.Prefix)
if err != nil {
log.Errorf("Failed to check if namespace already exists: %v", err)
ctx.JSON(http.StatusInternalServerError, gin.H{"error": "Server encountered an error checking if namespace already exists"})
Expand Down

0 comments on commit f8ac1da

Please sign in to comment.