Track persistence independent of presence of an identifier

Fixes a loop when adding `ActiveModel::Conversion` because conversion
defines `to_param` in terms of `persisted?`, which Valkyrie previously
defined in terms of `to_param`

Fixes #338

lib/valkyrie/persistence/fedora/persister/model_converter.rb

@@ -12,13 +12,21 @@ def initialize(resource:, adapter:, subject_uri: RDF::URI(""))
 
       def convert
         graph_resource.graph.delete([nil, nil, nil])
-        resource.attributes.each do |key, values|
-          output = property_converter.for(Property.new(subject_uri, key, values, adapter, resource)).result
+        properties.each do |property|
+          values = resource_attributes[property]
+
+          output = property_converter.for(Property.new(subject_uri, property, values, adapter, resource)).result
           graph_resource.graph << output.to_graph
         end
         graph_resource
       end
 
+      def properties
+        resource_attributes.keys - [:new_record]
+      end
+
+      delegate :attributes, to: :resource, prefix: true
+
       def graph_resource
         @graph_resource ||= ::Ldp::Container::Basic.new(connection, subject, nil, base_path)
       end

lib/valkyrie/persistence/fedora/persister/orm_converter.rb

@@ -14,8 +14,9 @@ def convert
       end
 
       def attributes
-        GraphToAttributes.new(graph: graph, adapter: adapter).convert.merge(id:
-                                                                            id)
+        GraphToAttributes.new(graph: graph, adapter: adapter)
+                         .convert
+                         .merge(id: id, new_record: false)
       end
 
       def id

lib/valkyrie/persistence/memory/persister.rb

@@ -41,7 +41,9 @@ def wipe!
     private
 
       def generate_id(resource)
-        resource.new(id: SecureRandom.uuid, created_at: Time.current)
+        resource.new(id: SecureRandom.uuid,
+                     created_at: Time.current,
+                     new_record: false)
       end
 
       def normalize_dates!(resource)

lib/valkyrie/persistence/postgres/orm_converter.rb

@@ -6,113 +6,129 @@ def initialize(orm_object)
       @orm_object = orm_object
     end
 
+    # Create a new instance of the class described in attributes[:internal_resource]
+    # and send it all the attributes that @orm_object has
     def convert!
-      @resource ||= attributes[:internal_resource].constantize.new(attributes)
+      @resource ||= resource
     end
 
-    # @return [Hash] Valkyrie-style hash of attributes.
-    def attributes
-      @attributes ||= orm_object.attributes.merge(rdf_metadata).symbolize_keys
-    end
+    private
 
-    def rdf_metadata
-      @rdf_metadata ||= RDFMetadata.new(orm_object.metadata).result
-    end
+      def resource
+        resource_klass.new(attributes.merge(new_record: false))
+      end
 
-    class RDFMetadata
-      attr_reader :metadata
-      def initialize(metadata)
-        @metadata = metadata
+      def resource_klass
+        internal_resource.constantize
       end
 
-      def result
-        Hash[
-          metadata.map do |key, value|
-            [key, PostgresValue.for(value).result]
-          end
-        ]
+      def internal_resource
+        attributes[:internal_resource]
       end
 
-      class PostgresValue < ::Valkyrie::ValueMapper
+      # @return [Hash] Valkyrie-style hash of attributes.
+      def attributes
+        @attributes ||= orm_object.attributes.merge(rdf_metadata).symbolize_keys
       end
-      # Converts {RDF::Literal} typed-literals from JSON-LD stored into an
-      #   {RDF::Literal}
-      class HashValue < ::Valkyrie::ValueMapper
-        PostgresValue.register(self)
-        def self.handles?(value)
-          value.is_a?(Hash) && value["@value"]
-        end
 
-        def result
-          RDF::Literal.new(value["@value"], language: value["@language"])
-        end
+      def rdf_metadata
+        @rdf_metadata ||= RDFMetadata.new(orm_object.metadata).result
       end
 
-      # Converts stored IDs into {Valkyrie::ID}s
-      class IDValue < ::Valkyrie::ValueMapper
-        PostgresValue.register(self)
-        def self.handles?(value)
-          value.is_a?(Hash) && value["id"] && !value["internal_resource"]
+      class RDFMetadata
+        attr_reader :metadata
+        def initialize(metadata)
+          @metadata = metadata
         end
 
         def result
-          Valkyrie::ID.new(value["id"])
+          Hash[
+            metadata.map do |key, value|
+              [key, PostgresValue.for(value).result]
+            end
+          ]
         end
-      end
 
-      # Converts stored URIs into {RDF::URI}s
-      class URIValue < ::Valkyrie::ValueMapper
-        PostgresValue.register(self)
-        def self.handles?(value)
-          value.is_a?(Hash) && value["@id"]
+        class PostgresValue < ::Valkyrie::ValueMapper
         end
+        # Converts {RDF::Literal} typed-literals from JSON-LD stored into an
+        #   {RDF::Literal}
+        class HashValue < ::Valkyrie::ValueMapper
+          PostgresValue.register(self)
+          def self.handles?(value)
+            value.is_a?(Hash) && value["@value"]
+          end
 
-        def result
-          ::RDF::URI.new(value["@id"])
+          def result
+            RDF::Literal.new(value["@value"], language: value["@language"])
+          end
         end
-      end
 
-      # Converts nested records into {Valkyrie::Resource}s
-      class NestedRecord < ::Valkyrie::ValueMapper
-        PostgresValue.register(self)
-        def self.handles?(value)
-          value.is_a?(Hash) && value.keys.length > 1
-        end
+        # Converts stored IDs into {Valkyrie::ID}s
+        class IDValue < ::Valkyrie::ValueMapper
+          PostgresValue.register(self)
+          def self.handles?(value)
+            value.is_a?(Hash) && value["id"] && !value["internal_resource"]
+          end
 
-        def result
-          RDFMetadata.new(value).result.symbolize_keys
+          def result
+            Valkyrie::ID.new(value["id"])
+          end
         end
-      end
 
-      class DateValue < ::Valkyrie::ValueMapper
-        PostgresValue.register(self)
-        def self.handles?(value)
-          return false unless value.is_a?(String)
-          return false unless value[4] == "-"
-          year = value.to_s[0..3]
-          return false unless year.length == 4 && year.to_i.to_s == year
-          DateTime.iso8601(value)
-        rescue
-          false
+        # Converts stored URIs into {RDF::URI}s
+        class URIValue < ::Valkyrie::ValueMapper
+          PostgresValue.register(self)
+          def self.handles?(value)
+            value.is_a?(Hash) && value["@id"]
+          end
+
+          def result
+            ::RDF::URI.new(value["@id"])
+          end
         end
 
-        def result
-          DateTime.iso8601(value).utc
+        # Converts nested records into {Valkyrie::Resource}s
+        class NestedRecord < ::Valkyrie::ValueMapper
+          PostgresValue.register(self)
+          def self.handles?(value)
+            value.is_a?(Hash) && value.keys.length > 1
+          end
+
+          def result
+            RDFMetadata.new(value).result.symbolize_keys
+          end
         end
-      end
 
-      class EnumeratorValue < ::Valkyrie::ValueMapper
-        PostgresValue.register(self)
-        def self.handles?(value)
-          value.respond_to?(:each)
+        class DateValue < ::Valkyrie::ValueMapper
+          PostgresValue.register(self)
+          def self.handles?(value)
+            return false unless value.is_a?(String)
+            return false unless value[4] == "-"
+            year = value.to_s[0..3]
+            return false unless year.length == 4 && year.to_i.to_s == year
+            DateTime.iso8601(value)
+          rescue
+            false
+          end
+
+          def result
+            DateTime.iso8601(value).utc
+          end
         end
 
-        def result
-          value.map do |value|
-            calling_mapper.for(value).result
+        class EnumeratorValue < ::Valkyrie::ValueMapper
+          PostgresValue.register(self)
+          def self.handles?(value)
+            value.respond_to?(:each)
+          end
+
+          def result
+            value.map do |value|
+              calling_mapper.for(value).result
+            end
           end
         end
       end
-    end
   end
 end

lib/valkyrie/persistence/solr/model_converter.rb

@@ -56,7 +56,7 @@ def attribute_hash
       end
 
       def properties
-        resource_attributes.keys - [:id, :created_at, :updated_at]
+        resource_attributes.keys - [:id, :created_at, :updated_at, :new_record]
       end
 
       def resource_attributes

lib/valkyrie/persistence/solr/orm_converter.rb

@@ -13,7 +13,7 @@ def convert!
     end
 
     def resource
-      resource_klass.new(attributes.symbolize_keys)
+      resource_klass.new(attributes.symbolize_keys.merge(new_record: false))
     end
 
     def resource_klass

lib/valkyrie/resource.rb

@@ -20,11 +20,12 @@ def self.inherited(subclass)
       subclass.attribute :internal_resource, Valkyrie::Types::Any.default(subclass.to_s)
       subclass.attribute :created_at, Valkyrie::Types::DateTime.optional
       subclass.attribute :updated_at, Valkyrie::Types::DateTime.optional
+      subclass.attribute :new_record, Types::Bool.default(true)
     end
 
     # @return [Array<Symbol>] Array of fields defined for this class.
     def self.fields
-      schema.keys
+      schema.keys.without(:new_record)
     end
 
     # Define an attribute.
@@ -75,7 +76,7 @@ def column_for_attribute(name)
 
     # @return [Boolean]
     def persisted?
-      to_param.present?
+      @new_record == false
     end
 
     def to_key

lib/valkyrie/specs/shared_specs/persister.rb

@@ -24,7 +24,10 @@ class CustomResource < Valkyrie::Resource
   it { is_expected.to respond_to(:delete).with_keywords(:resource) }
 
   it "can save a resource" do
-    expect(persister.save(resource: resource).id).not_to be_blank
+    expect(resource).not_to be_persisted
+    saved = persister.save(resource: resource)
+    expect(saved).to be_persisted
+    expect(saved.id).not_to be_blank
   end
 
   it "can save multiple resources at once" do

lib/valkyrie/specs/shared_specs/queries.rb

@@ -55,7 +55,9 @@ class SecondResource < Valkyrie::Resource
     it "returns a resource by id" do
       resource = persister.save(resource: resource_class.new)
 
-      expect(query_service.find_by(id: resource.id).id).to eq resource.id
+      found = query_service.find_by(id: resource.id)
+      expect(found.id).to eq resource.id
+      expect(found).to be_persisted
     end
 
     it "returns a Valkyrie::Persistence::ObjectNotFoundError for a non-found ID" do

spec/valkyrie/resource_spec.rb

@@ -32,12 +32,14 @@ class Resource < Valkyrie::Resource
   end
 
   describe "#persisted?" do
-    it "returns false if the ID is gone" do
-      expect(resource).not_to be_persisted
+    context 'when nothing is passed to the constructor' do
+      it { is_expected.not_to be_persisted }
     end
-    it "returns true if the ID exists" do
-      resource.id = "test"
-      expect(resource).to be_persisted
+
+    context 'when new_record: false is passed to the constructor' do
+      subject(:resource) { Resource.new(new_record: false) }
+
+      it { is_expected.to be_persisted }
     end
   end