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

feat(storage): internal GetProperties API #13869

57 changes: 57 additions & 0 deletions packages/storage/__tests__/internals/apis/getProperties.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import { AmplifyClassV6 } from '@aws-amplify/core';

import { getProperties as advancedGetProperties } from '../../../src/internals';
import { getProperties as getPropertiesInternal } from '../../../src/providers/s3/apis/internal/getProperties';

jest.mock('../../../src/providers/s3/apis/internal/getProperties');
const mockedGetPropertiesInternal = jest.mocked(getPropertiesInternal);

describe('getProperties (internal)', () => {
Copy link
Member

Choose a reason for hiding this comment

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

this is getting confusing 😅

beforeEach(() => {
mockedGetPropertiesInternal.mockResolvedValue({
path: 'output/path/to/mock/object',
});
});

afterEach(() => {
jest.clearAllMocks();
});

it('should pass advanced option locationCredentialsProvider to internal getProperties', async () => {
const useAccelerateEndpoint = true;
const bucket = { bucketName: 'bucket', region: 'us-east-1' };
const locationCredentialsProvider = async () => ({
credentials: {
accessKeyId: 'akid',
secretAccessKey: 'secret',
sessionToken: 'token',
expiration: new Date(),
},
});
const result = await advancedGetProperties({
path: 'input/path/to/mock/object',
options: {
useAccelerateEndpoint,
bucket,
locationCredentialsProvider,
},
});
expect(mockedGetPropertiesInternal).toHaveBeenCalledTimes(1);
expect(mockedGetPropertiesInternal).toHaveBeenCalledWith(
expect.any(AmplifyClassV6),
{
path: 'input/path/to/mock/object',
options: {
useAccelerateEndpoint,
bucket,
locationCredentialsProvider,
},
},
);
expect(result).toEqual({
path: 'output/path/to/mock/object',
});
});
});
33 changes: 33 additions & 0 deletions packages/storage/src/internals/apis/getProperties.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

import { Amplify } from '@aws-amplify/core';

import { getProperties as getPropertiesInternal } from '../../providers/s3/apis/internal/getProperties';
import { GetPropertiesInput } from '../types/inputs';
import { GetPropertiesOutput } from '../types/outputs';

/**
* Gets the properties of a file. The properties include S3 system metadata and
* the user metadata that was provided when uploading the file.
* @param input - The `GetPropertiesWithPathInput` object.
* @returns Requested object properties.
* @throws An `S3Exception` when the underlying S3 service returned error.
* @throws A `StorageValidationErrorCode` when API call parameters are invalid.
*
* @internal
*/
export function getProperties(
input: GetPropertiesInput,
): Promise<GetPropertiesOutput> {
return getPropertiesInternal(Amplify, {
path: input.path,
options: {
useAccelerateEndpoint: input?.options?.useAccelerateEndpoint,
bucket: input?.options?.bucket,
locationCredentialsProvider: input?.options?.locationCredentialsProvider,
ashika112 marked this conversation as resolved.
Show resolved Hide resolved
},
// Type casting is necessary because `getPropertiesInternal` supports both Gen1 and Gen2 signatures, but here
// given in input can only be Gen2 signature, the return can only ben Gen2 signature.
}) as Promise<GetPropertiesOutput>;
}
Comment on lines +20 to +33
Copy link
Member

Choose a reason for hiding this comment

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

super nit,

Suggested change
export function getProperties(
input: GetPropertiesInput,
): Promise<GetPropertiesOutput> {
return getPropertiesInternal(Amplify, {
path: input.path,
options: {
useAccelerateEndpoint: input?.options?.useAccelerateEndpoint,
bucket: input?.options?.bucket,
locationCredentialsProvider: input?.options?.locationCredentialsProvider,
},
// Type casting is necessary because `getPropertiesInternal` supports both Gen1 and Gen2 signatures, but here
// given in input can only be Gen2 signature, the return can only ben Gen2 signature.
}) as Promise<GetPropertiesOutput>;
}
export const getProperties = (
input: GetPropertiesInput
): Promise<GetPropertiesOutput> =>
getPropertiesInternal(Amplify, {
path: input.path,
options: {
useAccelerateEndpoint: input?.options?.useAccelerateEndpoint,
bucket: input?.options?.bucket,
locationCredentialsProvider: input?.options?.locationCredentialsProvider,
},
// Type casting is necessary because `getPropertiesInternal` supports both Gen1 and Gen2 signatures, but here
// given in input can only be Gen2 signature, the return can only ben Gen2 signature.
}) as Promise<GetPropertiesOutput>;

6 changes: 5 additions & 1 deletion packages/storage/src/internals/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

export { LocationCredentialsProvider } from '../providers/s3/types/options';
export { StorageSubpathStrategy } from '../types/options';

export { Permission } from './types/common';
Expand All @@ -12,14 +11,18 @@ Internal APIs
export {
GetDataAccessInput,
ListCallerAccessGrantsInput,
GetPropertiesInput,
CopyInput,
} from './types/inputs';
export {
GetDataAccessOutput,
ListCallerAccessGrantsOutput,
GetPropertiesOutput,
} from './types/outputs';

export { getDataAccess } from './apis/getDataAccess';
export { listCallerAccessGrants } from './apis/listCallerAccessGrants';
export { getProperties } from './apis/getProperties';

/*
CredentialsStore exports
Expand All @@ -41,4 +44,5 @@ export {
GetLocationCredentialsInput,
GetLocationCredentialsOutput,
} from './types/credentials';

export { AWSTemporaryCredentials } from '../providers/s3/types/options';
56 changes: 56 additions & 0 deletions packages/storage/src/internals/types/inputs.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

import {
StorageCopyInputWithPath,
StorageOperationInputWithPath,
StorageOperationOptionsInput,
} from '../../types/inputs';
import {
CopyWithPathInput,
GetPropertiesWithPathInput,
} from '../../providers/s3';

import { CredentialsProvider, ListLocationsInput } from './credentials';
import { Permission, PrefixType, Privilege } from './common';

Expand All @@ -26,3 +36,49 @@ export interface GetDataAccessInput {
region: string;
scope: string;
}

/**
* @internal
*/
export type GetPropertiesInput = ExtendInputWithAdvancedOptions<
Copy link
Member

Choose a reason for hiding this comment

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

Im confused here. Why are we creating this and not using this type? but rather have a duplicate again here?

Copy link
Member

Choose a reason for hiding this comment

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

+1, we should keep types as simple as we can, and avoid type programming as much as we can 😅 when we deprecating the key use cases, it should just simple splitting and deleting.

Copy link
Member Author

@AllanZhengYP AllanZhengYP Oct 2, 2024

Choose a reason for hiding this comment

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

@ashika112 I think I was debating all the different conventions, GetPropertiesInput name cannot be referred as-is in the other places. I can change to use this definition everywhere.

GetPropertiesWithPathInput,
{
locationCredentialsProvider?: CredentialsProvider;
}
>;

/**
* @internal
*/
export type CopyInput = ExtendCopyInputWithAdvancedOptions<
CopyWithPathInput,
{
locationCredentialsProvider?: CredentialsProvider;
}
>;

/**
* Generic types that extend the public non-copy API input types with extended
* options. This is a temporary solution to support advanced options from internal APIs.
*/
type ExtendInputWithAdvancedOptions<InputType, ExtendedOptionsType> =
InputType extends StorageOperationInputWithPath &
StorageOperationOptionsInput<infer PublicInputOptionsType>
? {
path: InputType['path'];
Copy link
Member

Choose a reason for hiding this comment

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

What about key? Not supporting it here?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, we only support Gen2 signature with extended option.

options?: PublicInputOptionsType & ExtendedOptionsType;
}
: never;

/**
* Generic types that extend the public copy API input type with extended options.
* This is a temporary solution to support advanced options from internal APIs.
*/
type ExtendCopyInputWithAdvancedOptions<InputType, ExtendedOptionsType> =
InputType extends StorageCopyInputWithPath
? {
source: InputType['source'];
destination: InputType['destination'];
options?: ExtendedOptionsType;
}
: never;
7 changes: 7 additions & 0 deletions packages/storage/src/internals/types/outputs.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

import { GetPropertiesWithPathOutput } from '../../providers/s3/types';

import { ListLocationsOutput, LocationCredentials } from './credentials';

/**
Expand All @@ -12,3 +14,8 @@ export type ListCallerAccessGrantsOutput = ListLocationsOutput;
* @internal
*/
export type GetDataAccessOutput = LocationCredentials;

/**
* @internal
*/
export type GetPropertiesOutput = GetPropertiesWithPathOutput;
6 changes: 4 additions & 2 deletions packages/storage/src/providers/s3/apis/internal/copy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import { assertValidationError } from '../../../../errors/utils/assertValidation
import { copyObject } from '../../utils/client/s3data';
import { getStorageUserAgentValue } from '../../utils/userAgent';
import { logger } from '../../../../utils';
// TODO: Remove this interface when we move to public advanced APIs.
import { CopyInput as CopyWithPathInputWithAdvancedOptions } from '../../../../internals';

const isCopyInputWithPath = (
input: CopyInput | CopyWithPathInput,
Expand All @@ -44,7 +46,7 @@ const storageBucketAssertion = (

export const copy = async (
amplify: AmplifyClassV6,
input: CopyInput | CopyWithPathInput,
input: CopyInput | CopyWithPathInputWithAdvancedOptions,
): Promise<CopyOutput | CopyWithPathOutput> => {
return isCopyInputWithPath(input)
? copyWithPath(amplify, input)
Expand All @@ -53,7 +55,7 @@ export const copy = async (

const copyWithPath = async (
amplify: AmplifyClassV6,
input: CopyWithPathInput,
input: CopyWithPathInputWithAdvancedOptions,
): Promise<CopyWithPathOutput> => {
const { source, destination } = input;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { StorageAction } from '@aws-amplify/core/internals/utils';
import {
GetPropertiesInput,
GetPropertiesOutput,
GetPropertiesWithPathInput,
GetPropertiesWithPathOutput,
} from '../../types';
import {
Expand All @@ -18,10 +17,12 @@ import { headObject } from '../../utils/client/s3data';
import { getStorageUserAgentValue } from '../../utils/userAgent';
import { logger } from '../../../../utils';
import { STORAGE_INPUT_KEY } from '../../utils/constants';
// TODO: Remove this interface when we move to public advanced APIs.
import { GetPropertiesInput as GetPropertiesWithPathInputWithAdvancedOptions } from '../../../../internals';

export const getProperties = async (
amplify: AmplifyClassV6,
input: GetPropertiesInput | GetPropertiesWithPathInput,
input: GetPropertiesInput | GetPropertiesWithPathInputWithAdvancedOptions,
action?: StorageAction,
): Promise<GetPropertiesOutput | GetPropertiesWithPathOutput> => {
const { s3Config, bucket, keyPrefix, identityId } =
Expand Down
8 changes: 1 addition & 7 deletions packages/storage/src/providers/s3/types/inputs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {
StorageUploadDataInputWithKey,
StorageUploadDataInputWithPath,
} from '../../../types';
import { StorageOperationOptionsInput } from '../../../types/inputs';
import {
CopyDestinationOptionsWithKey,
CopySourceOptionsWithKey,
Expand All @@ -36,8 +35,6 @@ import {
UploadDataOptionsWithPath,
} from '../types';

import { LocationCredentialsProvider } from './options';

// TODO: support use accelerate endpoint option
/**
* @deprecated Use {@link CopyWithPathInput} instead.
Expand All @@ -50,10 +47,7 @@ export type CopyInput = StorageCopyInputWithKey<
/**
* Input type with path for S3 copy API.
*/
export type CopyWithPathInput = StorageCopyInputWithPath &
StorageOperationOptionsInput<{
locationCredentialsProvider?: LocationCredentialsProvider;
}>;
export type CopyWithPathInput = StorageCopyInputWithPath;

/**
* @deprecated Use {@link GetPropertiesWithPathInput} instead.
Expand Down
14 changes: 6 additions & 8 deletions packages/storage/src/providers/s3/types/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
// SPDX-License-Identifier: Apache-2.0

import { StorageAccessLevel } from '@aws-amplify/core';
import { AWSCredentials } from '@aws-amplify/core/internals/utils';
import {
CredentialsProviderOptions,
SigningOptions,
} from '@aws-amplify/core/internals/aws-client-utils';
import { AWSCredentials } from '@aws-amplify/core/internals/utils';

import { TransferProgressEvent } from '../../../types';
import {
Expand All @@ -26,6 +26,11 @@ export type AWSTemporaryCredentials = Required<
>;

/**
* Async function returning AWS credentials for an API call. This function
* is invoked with S3 locations(bucket and path).
* If omitted, the global credentials configured in Amplify Auth
* would be used.
*
* @internal
*/
export type LocationCredentialsProvider = (
Expand All @@ -45,13 +50,6 @@ interface CommonOptions {
*/
useAccelerateEndpoint?: boolean;

/**
* Async function returning AWS credentials for an API call. This function
* is invoked with S3 locations(bucket and path).
* If omitted, the global credentials configured in Amplify Auth
* would be used.
*/
locationCredentialsProvider?: LocationCredentialsProvider;
bucket?: StorageBucket;
}

Expand Down
Loading