Add #id to ActiveFedora::File

This way File can be the target of ActiveFedora::Associations::RDF

lib/active_fedora/associations/rdf.rb

@@ -34,8 +34,9 @@ def filtered_results
       end
 
       # TODO Detect when this is the only relationship for this predicate, then skip the filtering.
+      # Subclasses of ActiveFedora::File are not filtered because they don't index their type in Solr
       def filtering_required?
-        reflection.klass != ActiveFedora::Base
+        !(reflection.klass == ActiveFedora::Base || reflection.klass <= ActiveFedora::File)
       end
 
       # @return [Array<RDF::URI>]

lib/active_fedora/fedora_attributes.rb

@@ -29,32 +29,13 @@ def set_value(*args)
       resource.set_value(*args)
     end
 
-    def id
-      if uri.kind_of?(::RDF::URI) && uri.value.blank?
-        nil
-      elsif uri.present?
-        self.class.uri_to_id(URI.parse(uri))
-      end
-    end
-
-    def id=(id)
-      raise "ID has already been set to #{self.id}" if self.id
-      @ldp_source = build_ldp_resource(id.to_s)
-    end
-
 
     # TODO: Remove after we no longer support #pid.
     def pid
       Deprecation.warn FedoraAttributes, "#{self.class}#pid is deprecated and will be removed in active-fedora 10.0. Use #{self.class}#id instead."
       id
     end
 
-    def uri
-      # TODO could we return a RDF::URI instead?
-      uri = @ldp_source.try(:subject_uri)
-      uri.value == '' ? uri : uri.to_s
-    end
-
     ##
     # The resource is the RdfResource object that stores the graph for
     # the datastream and is the central point for its relationship to

lib/active_fedora/file.rb

@@ -23,21 +23,17 @@ class File
     def initialize(parent_or_url_or_hash = nil, path=nil, options={})
       case parent_or_url_or_hash
       when Hash
-        content = ''
-        @ldp_source = Ldp::Resource::BinarySource.new(ldp_connection, nil, content, ActiveFedora.fedora.host + ActiveFedora.fedora.base_path)
+        build_ldp_resource_from_uri
       when nil, String
-      #TODO this is similar to Core#build_ldp_resource
-        content = ''
-        @ldp_source = Ldp::Resource::BinarySource.new(ldp_connection, parent_or_url_or_hash, content, ActiveFedora.fedora.host + ActiveFedora.fedora.base_path)
+        build_ldp_resource_from_uri(parent_or_url_or_hash)
       when ActiveFedora::Base
         Deprecation.warn File, "Initializing a file by passing a container is deprecated. Initialize with a uri instead. This capability will be removed in active-fedora 10.0"
         uri = if parent_or_url_or_hash.uri.kind_of?(::RDF::URI) && parent_or_url_or_hash.uri.value.empty?
           nil
         else
           "#{parent_or_url_or_hash.uri}/#{path}"
         end
-        @ldp_source = Ldp::Resource::BinarySource.new(ldp_connection, uri, nil, ActiveFedora.fedora.host + ActiveFedora.fedora.base_path)
-
+        build_ldp_resource_from_uri(uri)
       else
         raise "The first argument to #{self} must be a String or an ActiveFedora::Base. You provided a #{parent_or_url.class}"
       end
@@ -227,6 +223,16 @@ def fetch_mime_type
 
     private
 
+    # NOTE this is similar to Core#build_ldp_resource
+    def build_ldp_resource(id)
+      build_ldp_resource_from_uri(id_to_url(id))
+    end
+
+    def build_ldp_resource_from_uri(uri = nil)
+      content = ''
+      @ldp_source = Ldp::Resource::BinarySource.new(ldp_connection, uri, content, ActiveFedora.fedora.host + ActiveFedora.fedora.base_path)
+    end
+
     def links
       @links ||= Ldp::Response.links(ldp_source.head)
     end

lib/active_fedora/identifiable.rb

@@ -1,4 +1,6 @@
 module ActiveFedora
+  # This module depends on a instance variable @ldp_source
+  # and a method called build_ldp_resource
   module Identifiable
     extend ActiveSupport::Concern
 
@@ -54,5 +56,24 @@ def from_uri(uri,_)
         end
       end
     end
+
+    def id
+      if uri.kind_of?(::RDF::URI) && uri.value.blank?
+        nil
+      elsif uri.present?
+        self.class.uri_to_id(URI.parse(uri))
+      end
+    end
+
+    def id=(id)
+      raise "ID has already been set to #{self.id}" if self.id
+      @ldp_source = build_ldp_resource(id.to_s)
+    end
+
+    def uri
+      # TODO could we return a RDF::URI instead?
+      uri = @ldp_source.try(:subject_uri)
+      uri.value == '' ? uri : uri.to_s
+    end
   end
 end

spec/unit/associations/rdf_spec.rb

@@ -0,0 +1,42 @@
+require 'spec_helper'
+
+describe ActiveFedora::Associations::RDF do
+  let(:owner) { double }
+  let(:reflection) { double(check_validity!: true) }
+  let(:association) { described_class.new(owner, reflection) }
+
+  describe "#filtering_required?" do
+    before do
+      class SpecialFile < ActiveFedora::File; end
+      class SpecialObject < ActiveFedora::Base; end
+      allow(reflection).to receive(:klass).and_return(klass)
+    end
+
+    after do
+      Object.send(:remove_const, :SpecialFile)
+      Object.send(:remove_const, :SpecialObject)
+    end
+
+    subject { association.send(:filtering_required?) }
+
+    context "when the klass is ActiveFedora::Base" do
+      let(:klass) { ActiveFedora::Base }
+      it { is_expected.to be false }
+    end
+
+    context "when the klass is not ActiveFedora::Base" do
+      let(:klass) { SpecialObject }
+      it { is_expected.to be true }
+    end
+
+    context "when the klass is ActiveFedora::File" do
+      let(:klass) { ActiveFedora::File }
+      it { is_expected.to be false }
+    end
+
+    context "when the klass is a subklass of ActiveFedora::File" do
+      let(:klass) { SpecialFile }
+      it { is_expected.to be false }
+    end
+  end
+end

spec/unit/file_spec.rb

@@ -37,6 +37,12 @@
     end
   end
 
+  describe "#id" do
+    let(:file) { ActiveFedora::File.new("#{ActiveFedora.fedora.host}#{ActiveFedora.fedora.base_path}/1234/FOO1") }
+    subject { file.id }
+    it { is_expected.to eq '1234/FOO1' }
+  end
+
   context "content" do
     let(:mock_conn) do
       Faraday.new do |builder|