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

Restructure PackageService to better support endpoints requiring all versions of an ID #6905

Merged
merged 10 commits into from Feb 20, 2019

Conversation

scottbommarito
Copy link
Contributor

precursor to #6773

Both PackagesController.DisplayPackage and PackagesController.Manage require all versions of a package.

Previously, this was done either implicitly or lazily, specifically:

  • if a version isn't specified or is missing, FindPackageByIdAndVersion must fetch all versions to calculate the latest one
  • if a version was specified and found, the remaining versions would be loaded lazily

This is bad because our perf for these endpoints are relying on a hidden behavior of one of our services, and when this hidden behavior doesn't go our way, lazy loading is very very very very slow, and adding the deprecation entities associated with this feature makes it even worse.

So, to remedy this

  • add FindPackagesById to get all packages by ID (with a withDeprecations parameter for when deprecation entities are needed)
  • move logic from FindPackageByIdAndVersion that fetches the latest version from a list of packages and move it into FilterLatestPackage
    • FindPackageByIdAndVersion now calls FilterLatestPackage internally
  • make DisplayPackage and Manage call FindPackagesById and then FilterLatestPackage if an exact version isn't found
  • move the code from FindAbsoluteLatestPackageById into DisplayPackage

Copy link
Member

@joelverhagen joelverhagen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Address comment then :shipit:

Copy link
Contributor

@skofman1 skofman1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@scottbommarito scottbommarito merged commit 6647fe1 into dev Feb 20, 2019
@scottbommarito scottbommarito deleted the sb-findpackagesbyid branch February 20, 2019 20:47
@scottbommarito scottbommarito restored the sb-findpackagesbyid branch July 22, 2019 23:17
@scottbommarito scottbommarito deleted the sb-findpackagesbyid branch July 22, 2019 23:20
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.

None yet

5 participants