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

fix: Duplicated hierarchies #1260

Merged
merged 16 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
You can also check the [release page](https://github.com/visualize-admin/visualization-tool/releases)

## Unreleased

- Features
- Match drag and drop behavior for table chart and filter panel
- Fixes
- Remove ErrorWhisker and Tooltip when error is null in Column charts
- We now only fetch hierarchies defined in cube's shape
- Hierarchy names are now correctly retrieved
- Performance
- We no longer fetch shape when initalizing the cube, as we might need to re-fetch it again if a newer cube is required

# [3.24.0] - 2023-11-08

Expand Down
12 changes: 11 additions & 1 deletion app/graphql/context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { IncomingMessage } from "http";

import DataLoader from "dataloader";
import { GraphQLResolveInfo } from "graphql";
import rdf from "rdf-ext";
import StreamClient from "sparql-http-client";
import ParsingClient from "sparql-http-client/ParsingClient";
import { LRUCache } from "typescript-lru-cache";
Expand All @@ -10,6 +11,7 @@ import { SPARQL_GEO_ENDPOINT } from "@/domain/env";
import { Awaited } from "@/domain/types";
import { Timings } from "@/gql-flamegraph/resolvers";
import { createSource } from "@/rdf/create-source";
import { ExtendedCube } from "@/rdf/extended-cube";
import { timed, TimingCallback } from "@/utils/timed";

import { createCubeDimensionValuesLoader } from "../rdf/queries";
Expand All @@ -26,7 +28,15 @@ export const MAX_BATCH_SIZE = 500;

export const getRawCube = async (sourceUrl: string, iri: string) => {
const source = createSource({ endpointUrl: sourceUrl });
return await source.cube(iri);
const cube = new ExtendedCube({
parent: source,
term: rdf.namedNode(iri),
source,
});
// Don't fetch shape yet, as we might need to fetch newer cube.
await cube.fetchCube();

return cube;
};

// const cachedGetRawCube = cachedWithTTL(
Expand Down
30 changes: 28 additions & 2 deletions app/graphql/resolvers.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { IncomingMessage } from "http";
import { GraphQLResolveInfo } from "graphql";

import { createSource as createSource_ } from "@/rdf/create-source";
import { ExtendedCube as ExtendedCube_ } from "@/rdf/extended-cube";
import { getCubeObservations as getCubeObservations_ } from "@/rdf/queries";
import { unversionObservation as unversionObservation_ } from "@/rdf/query-dimension-values";

Expand All @@ -16,6 +17,10 @@ const createSource = createSource_ as unknown as jest.Mock<
typeof createSource_
>;

const ExtendedCube = ExtendedCube_ as unknown as jest.Mock<
typeof ExtendedCube_
>;

const unversionObservation = unversionObservation_ as unknown as jest.Mock<
typeof unversionObservation_
>;
Expand All @@ -27,12 +32,18 @@ jest.mock("../rdf/query-search", () => ({}));
jest.mock("../rdf/queries", () => ({
getCubeObservations: jest.fn(),
createCubeDimensionValuesLoader: () => async () => [],
getLatestCube: () => ({
fetchShape: () => ({}),
}),
}));
jest.mock("../rdf/create-source", () => ({
createSource: jest.fn(),
}));
jest.mock("../rdf/query-hierarchies", () => ({}));
jest.mock("../rdf/parse", () => ({}));
jest.mock("../rdf/extended-cube", () => ({
ExtendedCube: jest.fn(),
}));

jest.mock("@rdf-esm/data-model", () => ({}));
jest.mock("@rdf-esm/term-map", () => ({}));
Expand All @@ -45,6 +56,7 @@ describe("possible filters", () => {
beforeEach(() => {
getCubeObservations.mockReset();
});

it("should try to find an observation given possible filters, relaxing fitlers from the bottom", async () => {
// @ts-ignore
getCubeObservations.mockImplementation(async ({ filters }) => {
Expand Down Expand Up @@ -74,6 +86,11 @@ describe("possible filters", () => {
createSource.mockImplementation(() => ({
cube: () => ({}),
}));
// @ts-ignore
ExtendedCube.mockImplementation(() => ({
fetchCube: () => ({}),
fetchShape: () => ({}),
}));

const res = await Query?.possibleFilters?.(
{},
Expand All @@ -94,9 +111,18 @@ describe("possible filters", () => {
},
} as unknown as GraphQLResolveInfo
);

expect(res).toEqual([
{ iri: "https://fake-dimension-iri-1", type: "single", value: 1 },
{ iri: "https://fake-dimension-iri-2", type: "single", value: 3 },
{
iri: "https://fake-dimension-iri-1",
type: "single",
value: 1,
},
{
iri: "https://fake-dimension-iri-2",
type: "single",
value: 3,
},
]);
expect(getCubeObservations).toHaveBeenCalledTimes(2);
});
Expand Down
31 changes: 18 additions & 13 deletions app/graphql/resolvers/rdf.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@ import {
Resolvers,
SearchCubeResultOrder,
} from "@/graphql/resolver-types";
import { defaultLocale } from "@/locales/locales";
import { parseCube } from "@/rdf/parse";
import {
createCubeDimensionValuesLoader,
getCubeDimensions,
getCubeObservations,
getResolvedCube,
getLatestCube,
} from "@/rdf/queries";
import { unversionObservation } from "@/rdf/query-dimension-values";
import { queryHierarchy } from "@/rdf/query-hierarchies";
Expand Down Expand Up @@ -73,21 +75,27 @@ export const searchCubes: NonNullable<QueryResolvers["searchCubes"]> = async (
};

export const dataCubeByIri: NonNullable<QueryResolvers["dataCubeByIri"]> =
async (_, { iri, locale, latest }, { setup }, info) => {
async (_, { iri, locale, latest = true }, { setup }, info) => {
const { loaders } = await setup(info);
const cube = await loaders.cube.load(iri);
const rawCube = await loaders.cube.load(iri);

if (!cube) {
if (!rawCube) {
throw new Error("Cube not found");
}

return getResolvedCube({ cube, locale: locale || "de", latest });
const cube = latest ? await getLatestCube(rawCube) : rawCube;
await cube.fetchShape();

return parseCube({ cube, locale: locale ?? defaultLocale });
};

export const possibleFilters: NonNullable<QueryResolvers["possibleFilters"]> =
async (_, { iri, filters }, { setup }, info) => {
const { sparqlClient, loaders, cache } = await setup(info);
const cube = await loaders.cube.load(iri);
const rawCube = await loaders.cube.load(iri);
// Currently we always default to the latest cube.
const cube = await getLatestCube(rawCube);
Copy link
Collaborator

Choose a reason for hiding this comment

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

For the splitted issue, getLatestCube could be put on ExtendedCube.

await cube.fetchShape();

if (!cube) {
return [];
Expand All @@ -114,16 +122,15 @@ export const possibleFilters: NonNullable<QueryResolvers["possibleFilters"]> =

const unversioned = await unversionObservation({
observation: obs[0],
cube: cube,
cube,
sparqlClient,
});
const result = Object.keys(filters).map((f) => ({

return Object.keys(filters).map((f) => ({
iri: f,
type: "single",
value: unversioned[f],
}));

return result;
}

return [];
Expand Down Expand Up @@ -251,15 +258,13 @@ export const hierarchy: NonNullable<DimensionResolvers["hierarchy"]> = async (
throw new Error("Could not find cube");
}

const res = await queryHierarchy(
return await queryHierarchy(
rdimension,
locale,
sparqlClient,
sparqlClientStream,
cache
);

return res;
};

export const dimensionValues: NonNullable<
Expand Down
10 changes: 6 additions & 4 deletions app/graphql/shared-types.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { Cube, CubeDimension } from "rdf-cube-view-query";
import { CubeDimension } from "rdf-cube-view-query";
import { Literal, NamedNode } from "rdf-js";

import { ExtendedCube } from "@/rdf/extended-cube";

import { Observation } from "../domain/data";

import { RelatedDimension } from "./query-hooks";
Expand All @@ -15,7 +17,7 @@ import {
/** Types shared by graphql-codegen and resolver code */

export type ResolvedDataCube = {
cube: Cube;
cube: ExtendedCube;
locale: string;
data: {
iri: string;
Expand All @@ -42,7 +44,7 @@ export type ResolvedDataCube = {
};

export type ResolvedDimension = {
cube: Cube;
cube: ExtendedCube;
dimension: CubeDimension;
locale: string;
data: {
Expand Down Expand Up @@ -73,7 +75,7 @@ export type ResolvedDimension = {
export type ResolvedMeasure = ResolvedDimension;

export type ResolvedObservationsQuery = {
cube: Cube;
cube: ExtendedCube;
locale: string;

data: {
Expand Down
8 changes: 4 additions & 4 deletions app/rdf/cube-filters.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { Cube } from "rdf-cube-view-query";
import rdf from "rdf-ext";
import { NamedNode } from "rdf-js";

import { truthy } from "@/domain/types";
import { SearchCubeFilter } from "@/graphql/resolver-types";
import { ExtendedCube } from "@/rdf/extended-cube";
import * as ns from "@/rdf/namespace";

import isAttrEqual from "../utils/is-attr-equal";
Expand All @@ -22,7 +22,7 @@ export const makeInQueryFilter = (
filters: SearchCubeFilter[]
) => {
return filters.length > 0
? Cube.filter.in(
? ExtendedCube.filter.in(
predicate,
filters.map((x) => rdf.namedNode(x.value))
)
Expand Down Expand Up @@ -51,11 +51,11 @@ export const makeCubeFilters = ({

const res = [
// Cubes that have a newer version published have a schema.org/expires property; Only show cubes that don't have it
Cube.filter.noExpires(),
ExtendedCube.filter.noExpires(),
isVisualizeCubeFilter,
includeDrafts
? null
: Cube.filter.status([
: ExtendedCube.filter.status([
ns.adminVocabulary("CreativeWorkStatus/Published"),
]),
themeQueryFilter,
Expand Down
29 changes: 29 additions & 0 deletions app/rdf/extended-cube.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import clownface, { AnyPointer } from "clownface";
import { Cube, CubeOptions } from "rdf-cube-view-query";
import rdf from "rdf-ext";
import DatasetExt from "rdf-ext/lib/Dataset";

export class ExtendedCube extends Cube {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Cna you add a docstring describing the general goal ? So that when we see a method that should be there, we can be sure that this class is the right one ?

private shapeDataset: DatasetExt;
public shapePtr: AnyPointer;

constructor(options: CubeOptions) {
const { term = rdf.blankNode(), graph } = options;
super(options);
this.shapeDataset = rdf.dataset();
this.shapePtr = clownface({
term,
dataset: this.shapeDataset,
graph,
});
}

async fetchShape() {
const shapeData = await this.source.client.query.construct(
this.shapeQuery()
);
this.dataset.addAll(shapeData);
this.shapeDataset.addAll(shapeData);
this.quads = [...this.quads, ...shapeData];
}
}
13 changes: 7 additions & 6 deletions app/rdf/parse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,12 @@ import {
timeWeek,
timeYear,
} from "d3";
import { Cube, CubeDimension } from "rdf-cube-view-query";
import { CubeDimension } from "rdf-cube-view-query";
import { NamedNode, Term } from "rdf-js";

import { truthy } from "@/domain/types";
import { ScaleType } from "@/graphql/query-hooks";
import { ExtendedCube } from "@/rdf/extended-cube";

import { DataCubePublicationStatus, TimeUnit } from "../graphql/resolver-types";
import { ResolvedDataCube, ResolvedDimension } from "../graphql/shared-types";
Expand All @@ -29,18 +30,18 @@ export const getQueryLocales = (locale: string): string[] => [
"",
];

export const isCubePublished = (cube: Cube): boolean =>
export const isCubePublished = (cube: ExtendedCube): boolean =>
cube
.out(ns.schema.creativeWorkStatus)
.terms.some((t) =>
t.equals(ns.adminVocabulary("CreativeWorkStatus/Published"))
);

export const parseVersionHistory = (cube: Cube) => {
export const parseVersionHistory = (cube: ExtendedCube) => {
return cube.in(ns.schema.hasPart)?.value;
};

export const parseIri = (cube: Cube) => {
export const parseIri = (cube: ExtendedCube) => {
return cube.term?.value ?? "[NO IRI]";
};

Expand All @@ -53,7 +54,7 @@ export const parseCube = ({
cube,
locale,
}: {
cube: Cube;
cube: ExtendedCube;
locale: string;
}): ResolvedDataCube => {
const outOpts = { language: getQueryLocales(locale) };
Expand Down Expand Up @@ -206,7 +207,7 @@ export const parseCubeDimension = ({
units,
}: {
dim: CubeDimension;
cube: Cube;
cube: ExtendedCube;
locale: string;
units?: Map<
string,
Expand Down
Loading
Loading