Nested provider

[janhalama]

Description

service-client returns invalid provider format (extended with url property). ONESDK added more strict validation of provider definition. ONESDK provider validation started to fail.

Meeting notes: https://www.notion.so/superface/Nested-provider-in-service-client-327dd338e51e4b5db4ff86427b67b089

Motivation and Context

Deliver valid provider format for ONESDK.

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [x] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• [ ] I have updated the documentation accordingly.
• [x] I have read the CONTRIBUTION_GUIDE document.
• [x] I haven't repeated the code. (DRY)
• [x] I have added tests to cover my changes.
• [x] All new and existing tests passed.

[freaz]

I was thinking, that it could be split to two functions:

type ProviderMetadata = {
  provider_id: string;
  url: string;
  published_at: Date;
  published_by: string;
  owner: string;
  owner_url: string;
};

// probably not used now, but to be consistent with rest of the methods
getProvider(): ProviderMetadata;

// where provider definition is needed this would be used
getProviderDefinition(): ProviderJson;

Don't take it as it should be this way. This is just an idea.

I think @janhalama @Jakub-Vacek @kysely you should agree on it.

[janhalama]

I was thinking, that it could be split to two functions... I think @janhalama @Jakub-Vacek @kysely you should agree on it.

I like the idea. Profile and maps works the same way.

I am not sure about the getProvidersList function, currently it returns ProviderJson, Air renders default service url from ProviderJson. I think it would be handy for Air to have metadata and provider json in getProvidersList response.

[Jakub-Vacek]

From CLI standpoint splitting would make a lot of sense:)

[kysely]

I don't see any added value in splitting the methods for fetching metadata and the definition in a single resource request, only added complexity.

From the frontend pov, what really matters is the list response. Is it going to return list of metadata (probably this?); or list of definitions; or the whole document; or are we going to split it to multiple list methods?

If we plan on eventually modifying the underlying backend API to split the metadata from definition, too, I think it might make sense to already prepare the client now, otherwise I don't really see the point.

[freaz]

@kysely https://superfaceai.slack.com/archives/C01PVQBACB1/p1634404119165700 is the motivation.

[janhalama]

Backend API will return whole document (metadata + provider json).

I agree with @kysely not to add complexity (be transparent with backend api) and return whole document.

[kysely]

I'm familiar with the thread. Doesn't 'response.provider' now solve this?

[freaz]

and removing url for current and fixing retyping https://github.com/superfaceai/service-client/blob/main/src/client.ts#L272

🤷 feels forth fixing it properly.

[janhalama]

and removing url for current and fixing retyping https://github.com/superfaceai/service-client/blob/main/src/client.ts#L272

@freaz I did removed url for current (flat) provider format, but there is still retyping in mapping function. Not sure how to avoid it.

[freaz]

I am not right person to review.

[janhalama]

@Jakub-Vacek could you please also go through this PR

[Jakub-Vacek]

@janhalama not terribly happy with some of lint ignores but that's only temporary solution. You can merge it :)