Skip to content

Packages to use composition, not inheritance [RUN AS-IF-FOSS]

Alex Kalderimis requested to merge ajk-graphql-packages into master

What does this MR do?

GraphQL does not have inheritance, so it is an anti-pattern to use it to model polymorphism in our models.

The correct approach is to model polymorphism as implementation of interfaces. Here there is a single PackageType interface, and then different implementations of that.

in the course of implementing this, https://gitlab.com/gitlab-org/gitlab/-/issues/299371 was discovered.

Changes

This MR makes the following changes to the GraphQL schema:

  • Query.package_composer_details is removed, and replaced by Query.package. (breaking change).

    The old field was not type-safe, and was prone to runtime errors.

  • Package.versions.edges.node now has the type PackageWithoutVersions, which is identical to Package, except that it does not have a versions field. (breaking change).

    The Package.versions.versions field was an example of cyclic mutual recursion that could have deleterious effects on server response time.

  • Package.metadata is added. This is a union, which currently has the single member ComposerMetadata.

  • The PackageComposerDetails type is removed. It should be replaced by references to Package. (potenially breaking change for systems that rely on code-generation).

Any queries that refer to PackageComposerDetails, Query.package_composer_details or Package.versions.versions will need to be updated.

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Alex Kalderimis

Merge request reports

Loading