Update to support active-fedora 6 - 8

[cbeer]

The major changes in AF 7+ that affect dor-services are:

• mediashelf-loggable (which monkey-patches all Objects with a #logger instance) is no longer included.
• AF8 adds a datastream prefix to indexed fields by default; while this is probably a good idea, I've disabled this behavior for now
• ActiveFedora::Base.find is now aware of class hierarchy, so you can't call Dor::Item.find on e.g. a Dor::Collection and have it work right.
• Upstream #to_solr no longer mutates its arguments.

Fortunately, the changes seem to be backwards compatible with AF 6.x. I don't know whether this should indicate a new major release or not.

[coveralls]

Coverage increased (+0.03%) to 87.916% when pulling 8c20c42 on af7 into c8dbfe1 on master.

[coveralls]

Coverage increased (+0.03%) to 87.916% when pulling 8c20c42 on af7 into c8dbfe1 on master.

[coveralls]

Coverage increased (+0.03%) to 87.916% when pulling caf1403 on af7 into c8dbfe1 on master.

[coveralls]

Coverage increased (+0.03%) to 87.916% when pulling caf1403 on af7 into c8dbfe1 on master.

[coveralls]

Coverage increased (+0.03%) to 87.916% when pulling caf1403 on af7 into c8dbfe1 on master.

[coveralls]

Coverage increased (+0.03%) to 87.916% when pulling 16503ac on af7 into c8dbfe1 on master.

[coveralls]

Coverage increased (+0.03%) to 87.916% when pulling 175e809 on af7 into c8dbfe1 on master.

[coveralls]

Coverage increased (+0.03%) to 87.916% when pulling 8f7a8c0 on af7 into c8dbfe1 on master.

[coveralls]

Coverage increased (+0.03%) to 87.916% when pulling 8f7a8c0 on af7 into c8dbfe1 on master.

[coveralls]

Coverage increased (+0.03%) to 87.916% when pulling 9b27087 on af7 into c8dbfe1 on master.

[coveralls]

Coverage increased (+0.03%) to 87.916% when pulling f630778 on af7 into c8dbfe1 on master.

[mejackreed]

When looking through this, things are looking good. I'm 👍 for merging, though its hard to answer the semver question without knowing the context of where this sits. @atz ?

[jmartin-sul]

i'd like to test this with argo (and maybe dor-indexing-app), esp if we're not going to bump dor-services to v6 (i'm not really sure either way at this point whether that seems like the right thing to do to me).

for instance:

ActiveFedora::Base.find is now aware of class hierarchy, so you can't call Dor::Item.find on e.g. a Dor::Collection and have it work right.

argo's definitely been guilty of this in the past, and while i can't think of a specific instance off-hand where it's still happening, i'd be a bit surprised if it's not at all anymore (ditto other dor-services consumers). that's not to say that should hold us up from this upgrade -- that's definitely bad behavior on argo's part, and should get fixed. but it'd be good to be aware of things like that (and it shouldn't be too hard to grep for specific object type loads). maybe we could deploy to stage and do a bulk reindexing of it's 80k or so objects, and see what errors we come up with?

after writing the above, i feel not-too-strongly like this should result in dor-services v6. though since we're currently supporting v4 and v5, that would mean either supporting 3 major versions, or dropping support for version 4. i don't think this is as big of a breaking change as v4 to v5, but that was a pretty massive overhaul, so that's not the best standard to use.

in addition to @atz, does @drh-stanford have opinions? who else has dealt lots with dor-services usage? @peetucket?

FWIW, i haven't actually had a chance to review the changes, just going on the PR description and jack's comment (and the count of changed files, ha).

[cbeer]

Note that I already have a PR that fixes Argo (at least the bits covered by tests): sul-dlss/argo#844.

[peetucket]

so the core issue is that consumers need to be sure of the object type they are loading and instantiate the correct object type? what if they don't know ahead of time but just have a druid? is there a generic object you can load?

[cbeer]

Yes, Dor.find (as I switched all these to) or ActiveFedora::Base.find both work just fine. The problem is when you use a specific class to try to load something that isn't that type of object (e.g. using Dor::Item.find to try to load a Dor::Collection).

[peetucket]

Ok, that's good to know. So that's a pretty easy upgrade path for someone who gets bitten. Let's put that in the release notes.

[drh-stanford]

one question I have is what are we doing about ENABLE_SOLR_UPDATES? this is an AF 6 feature to avoid saves calling update index and we seem to use it in a few deployments. (frankly I'm confused about its usage). does AF > 6 still support this constant? if not, then I think unpinning AF may be problematic for some deployments.

also, re: semver, would we be bumping dor-services to v6 or keeping it a minor release on 5.x for this PR?

[cbeer]

ENABLE_SOLR_UPDATES is still a thing (even in ActiveFedora master). We just don't need to test that it works in dor-services, as it is tested upstream just fine.

[jmartin-sul]

i didn't realize (until seeing it in slack this morning) that this is still compatible with the older version of ActiveFedora. since there's still backwards compatibility, and since the upgrade path seems reasonably clear and not all that burdensome, i'm 👍 on this (having what looks like a good sweep of the affected behavior in argo is a big help too).

and the behavior that's not backwards compatible for consumers that upgrade ActiveFedora is a thing that should get fixed anyway, so it doesn't seem terrible that this provides pressure to make that change.

i'd vote for keeping this change on the 5.x line, since there's backwards compatibility. if the consumer just needs to pin something in Gemfile to keep from breaking on upgrade, it seems reasonable for that to fall under a minor version bump.

if it comes to that, i would also vote for trying to officially desupport v4 before we started supporting a v6. and i'd be in favor of bumping to v6 once AF6 is no longer supported by dor-services, since that does seem like a major breaking change (albiet not so much as a wholesale overhaul of what's indexed to what field names).