Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extracting Qa.authority_for #379

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

jeremyf
Copy link
Contributor

@jeremyf jeremyf commented Dec 5, 2022

Extracting Qa.authority_for

24776bc

Prior to this commit, we used different initialization logic for linked
data authorities and non-linked data. Further, the initialization logic
was locked away inside of a controller.

With this commit, we expose a method that provides consistent
initialization logic; albeit with different returned classes that have
slightly different implementation nuances. The application can then
rely on that and we can share initialization logic across controllers
and also expose a method that allows non-controller initialization to
begin to follow the same logic pathway.

Where are the tests? The initializations are covered by controller
tests, so I have chosen not to write new ones.

There is further work to do because LinkedData and non-LinkedData there
are different signatures for find and search. Ideally, we would
normalize the public facing implementation logic. However, this commit
preserves backwards compatibility, simply introducing a new feature.

Introducing Qa::AuthorityWrapper

baf399f

Prior to this commit, the returned value from Qa.authority_for was
something that was inconsistent. Depending on the controller (or authority), the find
and search would require different parameters.

With this commit, those nuances are encapsulated in their own methods.
This begins to work towards a more foundational promise of questioning
authority; namely that the end point that asked questions of QA didn't
need to know which type of authority it was.

That promise was partially delivered at the routes level but this commit
pushes that down to the internal API level.

As a bonus, and perhaps confusion, I've included the
Qa::AuthorityRequestContext as an interface to help convey how one
might provide a different (non-controller) context to instantiating and
authority.

WIP Reworking authority request context

4fd4907

require context and update logic of fetch/search header methods

d3c51a5

Adding a Qa::Authorities::Loc.type_for

79a13f9

Prior to this commit, we always assumed that each "find" would request
from https://id.loc.gov/authorities.

With this commit, we use the "subauthority" to inform what the base URL
is for the https://id.loc.gov/ host. In some cases this is
authorities and in others it's as the code indicates.

Hopefully works to close the following:

@jeremyf jeremyf force-pushed the jeremyf---extracting-logic-for-determining-qa-authority branch from 259042f to 300fb6e Compare December 5, 2022 22:06
Prior to this commit, we used different initialization logic for linked
data authorities and non-linked data.  Further, the initialization logic
was locked away inside of a controller.

With this commit, we expose a method that provides consistent
initialization logic; albeit with different returned classes that have
slightly different implementation nuances.  The application can then
rely on that and we can share initialization logic across controllers
and also expose a method that allows non-controller initialization to
begin to follow the same logic pathway.

Where are the tests?  The initializations are covered by controller
tests, so I have chosen not to write new ones.

There is further work to do because LinkedData and non-LinkedData there
are different signatures for `find` and `search`.  Ideally, we would
normalize the public facing implementation logic.  However, this commit
preserves backwards compatibility, simply introducing a new feature.
@jeremyf jeremyf force-pushed the jeremyf---extracting-logic-for-determining-qa-authority branch from f359447 to 24776bc Compare December 7, 2022 18:14
@jeremyf jeremyf marked this pull request as ready for review December 7, 2022 18:15
@jeremyf jeremyf changed the title WIP - Extracting Qa.authority_for Extracting Qa.authority_for Dec 7, 2022
jeremyf and others added 3 commits December 7, 2022 15:47
Prior to this commit, the returned value from `Qa.authority_for` was
something that was inconsistent.  Depending on the controller (or authority), the `find`
and `search` would require different parameters.

With this commit, those nuances are encapsulated in their own methods.
This begins to work towards a more foundational promise of questioning
authority; namely that the end point that asked questions of QA didn't
need to know which type of authority it was.

That promise was partially delivered at the routes level but this commit
pushes that down to the internal API level.

As a bonus, and perhaps confusion, I've included the
`Qa::AuthorityRequestContext` as an interface to help convey how one
might provide a different (non-controller) context to instantiating and
authority.
@jeremyf jeremyf force-pushed the jeremyf---extracting-logic-for-determining-qa-authority branch from e962cf8 to 1ff3d8a Compare December 9, 2022 20:06
Prior to this commit, we always assumed that each "find" would request
from `https://id.loc.gov/authorities`.

With this commit, we use the "subauthority" to inform what the base URL
is for the `https://id.loc.gov/` host.  In some cases this is
`authorities` and in others it's as the code indicates.

Hopefully works to close the following:

- scientist-softserv/utk-hyku#267
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants