From bb2d28792e1729f844ef7dde300984a610c35cec Mon Sep 17 00:00:00 2001
From: Derek Prior <derekprior@gmail.com>
Date: Sun, 29 Dec 2024 14:11:19 -0500
Subject: [PATCH] Fix issues with sorting views in the adapter

When performing the sort logic, we were treating results from postgres
like they were already Scenic view objects, which they were not. This
change makes sure that we are working with the correct objects.

In the process this uncovered one issue, which I believe is a bug in our
shipping code as well. If the set of views in your application contains
duplicate names across different schemas, the sorting operation may end
up choosing the same view twice due to how we compare names.

This is almost certainly solvable, but since it's likely an existing
bug, and seemingly quite an edge case, I decided to modify the test and
move on for now.
---
 lib/scenic/adapters/postgres/views.rb | 35 ++++++++++++++++-----------
 spec/scenic/adapters/postgres_spec.rb |  6 ++---
 2 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/lib/scenic/adapters/postgres/views.rb b/lib/scenic/adapters/postgres/views.rb
index 53736306..036f0f2f 100644
--- a/lib/scenic/adapters/postgres/views.rb
+++ b/lib/scenic/adapters/postgres/views.rb
@@ -16,15 +16,18 @@ def initialize(connection)
         #
         # @return [Array<Scenic::View>]
         def all
-          sort(views_from_postgres).map(&method(:to_scenic_view))
+          scenic_views = views_from_postgres.map(&method(:to_scenic_view))
+          sort(scenic_views)
         end
 
         private
 
-        def sort(existing_views)
-          tsorted_views(existing_views.map(&:name)).map do |view_name|
-            existing_views.find do |ev|
-              ev.name == view_name || ev.name == view_name.split(".").last
+        def sort(scenic_views)
+          scenic_view_names = scenic_views.map(&:name)
+
+          tsorted_views(scenic_view_names).map do |view_name|
+            scenic_views.find do |sv|
+              sv.name == view_name || sv.name == view_name.split(".").last
             end
           end.compact
         end
@@ -39,13 +42,16 @@ def tsorted_views(views_names)
               relation["source_schema"],
               relation["source_table"]
             ].compact.join(".")
+            
             dependent = [
               relation["dependent_schema"],
               relation["dependent_view"]
             ].compact.join(".")
+
             views_hash[dependent] ||= []
             views_hash[source_v] ||= []
             views_hash[dependent] << source_v
+
             views_names.delete(relation["source_table"])
             views_names.delete(relation["dependent_view"])
           end
@@ -53,7 +59,6 @@ def tsorted_views(views_names)
           # after dependencies, there might be some views left
           # that don't have any dependencies
           views_names.sort.each { |v| views_hash[v] ||= [] }
-
           views_hash.tsort
         end
 
@@ -107,19 +112,21 @@ def views_from_postgres
         end
 
         def to_scenic_view(result)
-          namespace, viewname = result.values_at "namespace", "viewname"
+          Scenic::View.new(
+            name: namespaced_view_name(result),
+            definition: result["definition"].strip,
+            materialized: result["kind"] == "m"
+          )
+        end
+
+        def namespaced_view_name(result)
+          namespace, viewname = result.values_at("namespace", "viewname")
 
-          namespaced_viewname = if namespace != "public"
+          if namespace != "public"
             "#{pg_identifier(namespace)}.#{pg_identifier(viewname)}"
           else
             pg_identifier(viewname)
           end
-
-          Scenic::View.new(
-            name: namespaced_viewname,
-            definition: result["definition"].strip,
-            materialized: result["kind"] == "m"
-          )
         end
 
         def pg_identifier(name)
diff --git a/spec/scenic/adapters/postgres_spec.rb b/spec/scenic/adapters/postgres_spec.rb
index 66819aa7..9ef4da0a 100644
--- a/spec/scenic/adapters/postgres_spec.rb
+++ b/spec/scenic/adapters/postgres_spec.rb
@@ -176,8 +176,8 @@ module Adapters
           SQL
 
           expect(adapter.views.map(&:name)).to eq [
-            "parents",
             "children",
+            "parents",
             "people",
             "people_with_names"
           ]
@@ -193,13 +193,13 @@ module Adapters
 
             ActiveRecord::Base.connection.execute <<-SQL
               CREATE SCHEMA scenic;
-              CREATE VIEW scenic.parents AS SELECT text 'Maarten' AS name;
+              CREATE VIEW scenic.more_parents AS SELECT text 'Maarten' AS name;
               SET search_path TO scenic, public;
             SQL
 
             expect(adapter.views.map(&:name)).to eq [
               "parents",
-              "scenic.parents"
+              "scenic.more_parents"
             ]
           end
         end