Skip to content

Remediate the severely messed up "GitBlob implements TreeEntry" inheritence structure

Created by: slimsag

I actually started out by trying to correct this - because the issue here meant I had introduced some nasty GraphQL complexity for API docs. I spent upwards of 6 hours trying to decipher all of this and get to a solution before realizing this is a bigger problem than I can immediately solve, so instead we get this issue.

Picture how you think an interface to a Git repository's files and directories might be structured. Maybe you envision something like this:

image

If that's what you are picturing roughly, I would agree with you. If only it was that simple. Our structure is insane, and inconsistent even with itself in multiple ways.

How you might think our API looks

First, let's look at a repository commit: we should have the ability to navigate the virtual filesystem that is a repository, right? You might imagine our API as being something like:

image

But that's not right

How our API actually looks

image

"Okay, whatever" you might think. "at least the API under those is sane, right?" well, no, here's what our actual API looks like:

image

Wild inconsistencies and spaghetti made of worms all over the place

  • File2 apparently has been a "temporary" name for at least as long as Sourcegraph has been open source. Nobody ever fixed it.
  • There is a new File type that is deprecated, but it's still used for SearchSuggestions
  • A TreeEntry describes "a file or directory", but a GitTree describes ONLY a directory according to the docstrings.
  • We can have file-specific resolvers via GitBlob, but it's not possible to have directory-specific resolvers: GitTree describes "ONLY" a directory - but by implementing the TreeEntry interface which describes a directory or file.
  • type Symbol has a location: Location! field which MUST describe the position of a symbol within a file, a GitBlob! and a Range - but yet it is implemented as being backed by a GitTreeEntryResolver which allegedly CANNOT be a file according to the type definition docstrings.
  • VirtualFile is in use in the graphqlbackend code, and is exposed through FileDiff APIs, but is not declared as in use by schema.graphql anywhere.

Mixins make the problem worse

Because we have no effective distinction between what is actually a file and what is actually a directory - e.g. we cannot have directory-specific resolvers at this time - there are various "mixins" like the ones in codeintel.schema that perpetuate this problem of "everything and nothing is both a file and a directory, maybe"

For example, the API docs resolvers mostly just need to be directory-centric, but we do not have (and cannot currently have) a directory-centric resolver: GitTreeEntryResolver is declared as being potentially a file, even though it implicitly is supposed to never be. Thus, API docs resolvers are also defined on files despite the fact that attempting to use them on a file would always result in an error.

Untangling this is made harder by the fact that e.g. we've chosen in the codeintel backend to implement GitTreeEntryResolver using a GitBlobEntryResolver(!) and it is not possible to merely undo this because doing so means we must first untangle the fact that a tree entry MUST also be a blob entry as stated earlier:

  • type Symbol has a location: Location! field which MUST describe the position of a symbol within a file, a GitBlob! and a Range - but yet it is implemented as being backed by a GitTreeEntryResolver which allegedly CANNOT be a file according to the type definition docstrings.

If you don't believe me, try it:

15:09:09                  frontend | fatal: graphqlbackend.GitTreeLSIFDataResolver does not resolve "GitBlobLSIFData": missing method for field "ranges"
15:09:09                  frontend | 	used by (*graphqlbackend.GitTreeEntryResolver).LSIF
15:09:09                  frontend | 	used by (*graphqlbackend.locationResolver).Resource
15:09:09                  frontend | 	used by (graphqlbackend.symbolResolver).Location
15:09:09                  frontend | 	used by (*graphqlbackend.symbolConnectionResolver).Nodes
15:09:09                  frontend | 	used by (*graphqlbackend.GitCommitResolver).Symbols
15:09:09                  frontend | 	used by (*graphqlbackend.gitCommitConnectionResolver).Nodes
15:09:09                  frontend | 	used by (*graphqlbackend.GitCommitResolver).Ancestors
15:09:09                  frontend | 	used by (*graphqlbackend.GitTreeEntryResolver).Commit
15:09:09                  frontend | 	used by (*graphqlbackend.GitCommitResolver).Blob

We're going to need to put in a lot of work to.. correct this.

note-to-self: sg/apidocs-doc-resolvers