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(odata-service-inquirer): #2309 fix for annotations not retrieved using service url prompt #2310

Merged
merged 4 commits into from
Sep 2, 2024
Merged
Show file tree
Hide file tree
Changes from all 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 .changeset/bright-spiders-carry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sap-ux/odata-service-inquirer': patch
---

Fix for annotations not retrieved by service url prompt
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export class ConnectionValidator {

private _odataService: ODataService | undefined;
private _serviceProvider: ServiceProvider | undefined;
private _axiosConfig: (AxiosExtensionRequestConfig & ProviderConfiguration) | undefined;
private _axiosConfig: AxiosExtensionRequestConfig & ProviderConfiguration;
private _catalogV2: CatalogService | undefined;
private _catalogV4: CatalogService | undefined;
private _systemAuthType: SystemAuthType | undefined;
Expand All @@ -82,7 +82,7 @@ export class ConnectionValidator {
*
* @returns the axios configuration
*/
public get axiosConfig(): AxiosRequestConfig | undefined {
public get axiosConfig(): AxiosRequestConfig {
return this._axiosConfig;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@ import { t } from '../../../i18n';
import type { OdataServiceAnswers, OdataServicePromptOptions } from '../../../types';
import { hostEnvironment, promptNames } from '../../../types';
import { PromptState, getHostEnvironment } from '../../../utils';
import LoggerHelper from '../../logger-helper';
import { ConnectionValidator } from '../../connectionValidator';
import LoggerHelper from '../../logger-helper';
import { serviceUrlInternalPromptNames } from './types';
import { validateService } from './validators';
import type { AbapServiceProvider } from '@sap-ux/axios-extension';

/**
* Internal only answers to service URL prompting not returned with OdataServiceAnswers.
Expand Down Expand Up @@ -64,7 +63,7 @@ function getServiceUrlPrompt(connectValidator: ConnectionValidator, requiredVers
url,
{
odataService: connectValidator.odataService,
abapServiceProvider: connectValidator.serviceProvider as AbapServiceProvider
axiosConfig: connectValidator.axiosConfig
},
requiredVersion
);
Expand Down Expand Up @@ -118,7 +117,7 @@ function getIgnoreCertErrorsPrompt(
serviceUrl,
{
odataService: connectValidator.odataService,
abapServiceProvider: connectValidator.serviceProvider as AbapServiceProvider
axiosConfig: connectValidator.axiosConfig
},
requiredVersion,
ignoreCertError
Expand Down Expand Up @@ -167,7 +166,7 @@ function getCliIgnoreCertValidatePrompt(
serviceUrl,
{
odataService: connectValidator.odataService,
abapServiceProvider: connectValidator.serviceProvider as AbapServiceProvider
axiosConfig: connectValidator.axiosConfig
},
requiredVersion,
true
Expand Down Expand Up @@ -237,7 +236,7 @@ function getPasswordPrompt(
serviceUrl,
{
odataService: connectValidator.odataService,
abapServiceProvider: connectValidator.serviceProvider as AbapServiceProvider
axiosConfig: connectValidator.axiosConfig
},
requiredVersion,
ignoreCertError
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { AbapServiceProvider, ODataService, ODataVersion } from '@sap-ux/axios-extension';
import { createForAbap, type AxiosRequestConfig, type ODataService, type ODataVersion } from '@sap-ux/axios-extension';
import type { OdataVersion } from '@sap-ux/odata-service-writer';
import { ERROR_TYPE, ErrorHandler } from '../../../error-handler/error-handler';
import { t } from '../../../i18n';
Expand All @@ -15,14 +15,14 @@ import { errorHandler } from '../../prompt-helpers';
* @param url the full odata service url including query parameters
* @param connectionConfig the connection configuration to use for the validation, a subset of the ConnectionValidator properties
* @param connectionConfig.odataService the odata service instance used to retrieve the metadata (as used by ConnectionValidator)
* @param connectionConfig.abapServiceProvider the abap service provider instance used to retrieve annotations (as used by ConnectionValidator)
* @param connectionConfig.axiosConfig the axios config to use for the annotations request (as used by ConnectionValidator)
* @param requiredVersion if specified and the service odata version does not match this version, an error is returned
* @param ignoreCertError if true some certificate errors are ignored
* @returns true if a valid odata service was returned, false or an error message string otherwise
*/
export async function validateService(
url: string,
{ odataService, abapServiceProvider }: { odataService: ODataService; abapServiceProvider: AbapServiceProvider },
{ odataService, axiosConfig }: { odataService: ODataService; axiosConfig: AxiosRequestConfig },
requiredVersion: OdataVersion | undefined = undefined,
ignoreCertError = false
): Promise<boolean | string> {
Expand Down Expand Up @@ -56,7 +56,8 @@ export async function validateService(
// Best effort attempt to get annotations but dont throw an error if it fails as this may not even be an Abap system
try {
// Create an abap provider instance to get the annotations using the same request config
const catalogService = abapServiceProvider.catalog(serviceOdataVersion as unknown as ODataVersion);
const abapProvider = createForAbap(axiosConfig);
const catalogService = abapProvider.catalog(serviceOdataVersion as unknown as ODataVersion);
LoggerHelper.attachAxiosLogger(catalogService.interceptors);
LoggerHelper.logger.debug('Getting annotations for service');
const annotations = await catalogService.getAnnotations({ path: fullUrl.pathname });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ const connectionValidatorMock = {
validateUrl: validateUrlMockTrue,
validateAuth: validateAuthTrue,
odataService: odataServiceMock,
serviceProvider: serviceProviderMock
serviceProvider: serviceProviderMock,
axiosConfig: {}
};
jest.mock('../../../../src/prompts/connectionValidator', () => {
return {
Expand Down Expand Up @@ -129,6 +130,7 @@ describe('Service URL prompts', () => {
authRequired: false,
authenticated: false
};
connectionValidatorMock.axiosConfig = {};

// Should validate service and return true if valid
const serviceUrl = 'https://some.host:1234/service/path';
Expand All @@ -140,7 +142,7 @@ describe('Service URL prompts', () => {

expect(serviceValidatorSpy).toHaveBeenCalledWith(
serviceUrl,
expect.objectContaining({ 'abapServiceProvider': {}, 'odataService': {} }),
expect.objectContaining({ 'axiosConfig': {}, 'odataService': {} }),
undefined
);
expect(validateUrlMockTrue).toHaveBeenCalledWith(serviceUrl);
Expand All @@ -155,7 +157,7 @@ describe('Service URL prompts', () => {
expect(connectionValidatorMock.validateUrl).toHaveBeenCalledWith(serviceUrl);
expect(serviceValidatorSpy).toHaveBeenCalledWith(
serviceUrl,
expect.objectContaining({ 'abapServiceProvider': {}, 'odataService': {} }),
expect.objectContaining({ 'axiosConfig': {}, 'odataService': {} }),
OdataVersion.v4
);

Expand Down Expand Up @@ -243,7 +245,7 @@ describe('Service URL prompts', () => {
});
expect(serviceValidatorSpy).toHaveBeenCalledWith(
serviceUrl,
expect.objectContaining({ 'abapServiceProvider': {}, 'odataService': {} }),
expect.objectContaining({ 'axiosConfig': {}, 'odataService': {} }),
undefined,
true
);
Expand Down Expand Up @@ -321,7 +323,7 @@ describe('Service URL prompts', () => {
});
expect(serviceValidatorSpy).toHaveBeenCalledWith(
serviceUrl,
expect.objectContaining({ 'abapServiceProvider': {}, 'odataService': {} }),
expect.objectContaining({ 'axiosConfig': {}, 'odataService': {} }),
undefined,
true
);
Expand Down Expand Up @@ -400,7 +402,7 @@ describe('Service URL prompts', () => {
});
expect(serviceValidatorSpy).toHaveBeenCalledWith(
serviceUrl,
expect.objectContaining({ 'abapServiceProvider': {}, 'odataService': {} }),
expect.objectContaining({ 'axiosConfig': {}, 'odataService': {} }),
undefined,
undefined
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ jest.mock('@sap-ux/axios-extension', () => ({
...jest.requireActual('@sap-ux/axios-extension'),
AbapServiceProvider: jest.fn().mockImplementation(() => ({
catalog: catalogServiceMock
}))
})),
createForAbap: jest.fn().mockImplementation(() => new AbapServiceProvider())
}));

describe('Test service url validators', () => {
Expand Down Expand Up @@ -59,7 +60,7 @@ describe('Test service url validators', () => {
expect(
await validateService(serviceUrl, {
odataService,
abapServiceProvider: new AbapServiceProvider()
axiosConfig: {}
})
).toMatch(t('prompts.validationMessages.metadataInvalid'));

Expand All @@ -68,31 +69,19 @@ describe('Test service url validators', () => {
expect(
await validateService(serviceUrl, {
odataService,
abapServiceProvider: new AbapServiceProvider()
axiosConfig: {}
})
).toBe(true);
expect(catalogServiceMock).toHaveBeenCalledWith(OdataVersion.v2);

// Valid metadata with required version
expect(
await validateService(
serviceUrl,
{ odataService, abapServiceProvider: new AbapServiceProvider() },
OdataVersion.v4
)
).toBe(
expect(await validateService(serviceUrl, { odataService, axiosConfig: {} }, OdataVersion.v4)).toBe(
t('prompts.validationMessages.odataVersionMismatch', {
requiredOdataVersion: OdataVersion.v4,
providedOdataVersion: OdataVersion.v2
})
);
expect(
await validateService(
serviceUrl,
{ odataService, abapServiceProvider: new AbapServiceProvider() },
OdataVersion.v2
)
).toBe(true);
expect(await validateService(serviceUrl, { odataService, axiosConfig: {} }, OdataVersion.v2)).toBe(true);
});

test('should set the prompt state', async () => {
Expand All @@ -111,7 +100,7 @@ describe('Test service url validators', () => {
expect(
await validateService(serviceUrl, {
odataService,
abapServiceProvider: new AbapServiceProvider()
'axiosConfig': {}
})
).toBe(true);
expect(PromptState.odataService).toEqual({
Expand Down Expand Up @@ -140,7 +129,7 @@ describe('Test service url validators', () => {
expect(
await validateService(serviceUrl, {
odataService,
abapServiceProvider: new AbapServiceProvider()
'axiosConfig': {}
})
).toBe(true);
expect(loggerSpy).toHaveBeenCalledWith(t('prompts.validationMessages.annotationsNotFound'));
Expand All @@ -151,7 +140,7 @@ describe('Test service url validators', () => {
expect(
await validateService(serviceUrl, {
odataService,
abapServiceProvider: new AbapServiceProvider()
'axiosConfig': {}
})
).toBe(true);
expect(loggerSpy).toHaveBeenCalledWith(t('prompts.validationMessages.annotationsNotFound'));
Expand All @@ -167,7 +156,7 @@ describe('Test service url validators', () => {
expect(
await validateService(serviceUrl, {
odataService,
abapServiceProvider: new AbapServiceProvider()
'axiosConfig': {}
})
).toBe(t('errors.unknownError', { error: metadataRequestError.message }));
expect(loggerSpy).toHaveBeenCalled();
Expand All @@ -179,7 +168,7 @@ describe('Test service url validators', () => {
expect(
await validateService(serviceUrl, {
odataService,
abapServiceProvider: new AbapServiceProvider()
'axiosConfig': {}
})
).toBe(t('errors.odataServiceUrlNotFound'));
});
Expand Down
Loading