Merge pull request #2824 from Azure/shpaster/storage-blob-fix

Fixes to gallery and to storage functionality

desktop/python/server/aad_auth.py

@@ -1,5 +1,14 @@
 import azext.batch
 from msrestazure.azure_active_directory import AdalAuthentication
+from azure.core.credentials import TokenCredential, AccessToken
+
+class BatchExplorerTokenCredential(TokenCredential):
+    def __init__(self, access_token) -> None:
+        super().__init__()
+        self.access_token = access_token
+
+    def get_token(self, *scopes, **kwargs):
+        return AccessToken(token=self.access_token, expires_on=0)
 
 class BatchAccount:
     def __init__(self, account_id: str, name: str, account_endpoint: str, subscription_id: str):
@@ -22,8 +31,8 @@ class AADAuth:
     def __init__(self, batchToken: str, armToken: str, armUrl: str, storage_endpoint: str, account: BatchAccount):
         self.batchCreds = AdalAuthentication(
             lambda: {'accessToken': batchToken, 'tokenType': 'Bearer'})
-        self.armCreds = AdalAuthentication(
-            lambda: {'accessToken': armToken, 'tokenType': 'Bearer'})
+
+        self.armCreds = BatchExplorerTokenCredential(armToken)
         self.armUrl = armUrl
         self.storage_endpoint = storage_endpoint
         self.account = account

desktop/src/@batch-flask/core/data/targeted-data-cache/targeted-data-cache.ts

@@ -27,7 +27,7 @@ export class TargetedDataCache<TParams, TEntity extends Record<any>> {
      * Return the key of the cache associated to the given params
      */
     public getCacheKey(params: TParams) {
-        return this._options.key!(params);
+        return this._options.key?.call(null, params);
     }
 
     public getCache(params: TParams): DataCache<TEntity> {

desktop/src/@batch-flask/core/record/decorators.ts

@@ -56,13 +56,12 @@ export function ListProp<T>(type: any) {
     };
 }
 
-export function Model() {
+export function Model(name?: string) {
     return <T extends new(...args: any[]) => {}>(ctr: T) => {
         if (!(ctr.prototype instanceof Record)) {
             throw new RecordMissingExtendsError(ctr);
         }
-
-        return (class extends ctr {
+        const model = (class extends ctr {
             constructor(...args: any[]) {
                 const [data] = args;
                 if (data instanceof ctr) {
@@ -72,5 +71,7 @@ export function Model() {
                 (this as any)._completeInitialization();
             }
         });
+        Object.defineProperty(model, "name", { value: name || ctr.name });
+        return model;
     };
 }

desktop/src/@batch-flask/core/record/record.spec.ts

@@ -44,6 +44,12 @@ class InheritedTestRec extends SimpleTestRec {
     public d: number;
 }
 
+@Model("CustomName")
+class CustomNameRec extends Record<any> {
+    @Prop()
+    public id: string = "default-id";
+}
+
 describe("Record", () => {
     it("should throw an exeption when record doesn't extends Record class", () => {
         try {
@@ -176,4 +182,12 @@ describe("Record", () => {
         expect(SimpleTestRec.isStaticMethod).not.toBeFalsy();
         expect(SimpleTestRec.isStaticMethod()).toBe(true);
     });
+
+    it("should allow a custom record name to be set", () => {
+        const rec1 = new TestRec();
+        expect(rec1.constructor.name).toEqual("TestRec");
+
+        const rec2 = new CustomNameRec();
+        expect(rec2.constructor.name).toEqual("CustomName");
+    });
 });

desktop/src/app/components/data/shared/file-group-picker/file-group-picker.component.spec.ts

@@ -0,0 +1,92 @@
+import { HttpClientTestingModule } from "@angular/common/http/testing";
+import { Component, DebugElement } from "@angular/core";
+import { ComponentFixture, TestBed } from "@angular/core/testing";
+import { FormControl, FormsModule, ReactiveFormsModule } from "@angular/forms";
+import { By } from "@angular/platform-browser";
+import { BrowserDynamicTestingModule }
+    from "@angular/platform-browser-dynamic/testing";
+import { RouterTestingModule } from "@angular/router/testing";
+import { GlobalStorage, USER_SERVICE, UserConfigurationService }
+    from "@batch-flask/core";
+import {
+    I18nTestingModule,
+    MockGlobalStorage,
+    MockUserConfigurationService
+} from "@batch-flask/core/testing";
+import { ElectronTestingModule } from "@batch-flask/electron/testing";
+import { ButtonsModule, DialogService, FormModule, SelectModule }
+    from "@batch-flask/ui";
+import { AuthService, BatchExplorerService } from "app/services";
+import { BehaviorSubject } from "rxjs";
+import { FileGroupPickerComponent } from "./file-group-picker.component";
+import { FileGroupPickerModule } from "./file-group-picker.module";
+
+@Component({
+    template: `<bl-file-group-picker [formControl]="control"></bl-file-group-picker>`,
+})
+class TestComponent {
+    public control = new FormControl();
+}
+
+describe("FileGroupPickerComponent", () => {
+    let testComponent: TestComponent;
+    let component: FileGroupPickerComponent;
+    let fixture: ComponentFixture<TestComponent>;
+    let de: DebugElement;
+
+    const userServiceSpy = { currentUser: new BehaviorSubject(null) };
+
+    beforeEach(async () => {
+        await TestBed.configureTestingModule({
+            imports: [
+                FormModule,
+                FormsModule,
+                ReactiveFormsModule,
+                SelectModule,
+                RouterTestingModule,
+                I18nTestingModule,
+                HttpClientTestingModule,
+                ButtonsModule,
+                ElectronTestingModule,
+                BrowserDynamicTestingModule,
+                FileGroupPickerModule
+            ],
+            declarations: [TestComponent],
+            providers: [
+                { provide: BatchExplorerService, useValue: {} },
+                { provide: UserConfigurationService, useValue:
+                    new MockUserConfigurationService({}) },
+                { provide: AuthService, useValue: userServiceSpy },
+                { provide: GlobalStorage, useValue: new MockGlobalStorage() },
+                { provide: USER_SERVICE, useValue: userServiceSpy },
+                { provide: DialogService, useValue: {} },
+            ]
+        }).compileComponents();
+
+        fixture = TestBed.createComponent(TestComponent);
+        testComponent = fixture.componentInstance;
+        de = fixture.debugElement.query(By.css("bl-file-group-picker"));
+        component = fixture.debugElement.query(By.css("bl-file-group-picker"))
+            .componentInstance;
+        fixture.detectChanges();
+    });
+
+    it("should pick an existing value", () => {
+        const testValue = 'existingValue';
+        component.fileGroupPicked(testValue);
+        fixture.detectChanges();
+        expect(component.value.value).toEqual(testValue);
+    });
+
+    it("should create a new file group on null value", () => {
+        const spy = spyOn(component, "createFileGroup");
+        const testValue = null;
+        component.fileGroupPicked(testValue);
+        fixture.detectChanges();
+        expect(spy).toHaveBeenCalledOnce();
+    });
+
+    afterEach(() => {
+        fixture.destroy();
+    });
+});

desktop/src/app/components/data/shared/file-group-picker/file-group-picker.component.ts

@@ -4,7 +4,6 @@ import {
 import {
     ControlValueAccessor, FormControl, NG_VALIDATORS, NG_VALUE_ACCESSOR,
 } from "@angular/forms";
-import { MatOptionSelectionChange } from "@angular/material/core";
 import { FilterBuilder, ListView } from "@batch-flask/core";
 import { Activity, DialogService } from "@batch-flask/ui";
 import { FileGroupCreateFormComponent } from "app/components/data/action";
@@ -141,18 +140,25 @@ export class FileGroupPickerComponent implements ControlValueAccessor, OnInit, O
         return null;
     }
 
-    public createFileGroup(dropdownValue: string) {
-        if (!dropdownValue) {
-            const dialog = this.dialogService.open(FileGroupCreateFormComponent);
-            dialog.afterClosed().subscribe((activity?: Activity) => {
-                const newFileGroupName = dialog.componentInstance.getCurrentValue().name;
-                this.value.setValue(this.fileGroupService.addFileGroupPrefix(newFileGroupName));
-                this.changeDetector.markForCheck();
-                this._uploadActivity.next(activity);
-            });
+    public fileGroupPicked(value: string) {
+        if (value) {
+            this.writeValue(value);
+            this.changeDetector.markForCheck();
+        } else {
+            this.createFileGroup();
         }
     }
 
+    public createFileGroup() {
+        const dialog = this.dialogService.open(FileGroupCreateFormComponent);
+        dialog.afterClosed().subscribe((activity?: Activity) => {
+            const newFileGroupName = dialog.componentInstance.getCurrentValue().name;
+            this.value.setValue(this.fileGroupService.addFileGroupPrefix(newFileGroupName));
+            this.changeDetector.markForCheck();
+            this._uploadActivity.next(activity);
+        });
+    }
+
     public trackFileGroup(_: number, fileGroup: BlobContainer) {
         return fileGroup.id;
     }

desktop/src/app/components/data/shared/file-group-picker/file-group-picker.html

@@ -8,11 +8,12 @@
     <bl-hint *ngIf="hint" align="end">
         {{hint}}
     </bl-hint>
-    <bl-select placeholder="File group" [filterable]="true" (change)="createFileGroup($event)">
+    <bl-select placeholder="File group" [filterable]="true" (change)="fileGroupPicked($event)">
         <bl-option value="" label="+ {{'file-group-picker.create' | i18n}}"></bl-option>
         <bl-option
             *ngFor="let fileGroup of fileGroups;trackBy: trackFileGroup"
-            [value]="fileGroup">
+            [value]="fileGroup.name"
+            [label]="fileGroup.name">
         </bl-option>
     </bl-select>
 </bl-form-field>

desktop/src/app/components/gallery/submit/market-application.model.ts

@@ -38,9 +38,12 @@ export class NcjParameterWrapper {
     }
 
     private _computeDefaultValue() {
-        if (this.param.defaultValue) {
-            this.defaultValue = this.param.defaultValue;
+        let defaultValue = this.param.defaultValue;
+        if (typeof defaultValue === "string" &&
+            defaultValue.toLowerCase().trim() === "none") {
+            defaultValue = "";
         }
+        this.defaultValue = this.param.defaultValue = defaultValue;
     }
 
     private _computeDescription() {

desktop/src/app/components/gallery/submit/parameter-input/parameter-input.component.ts

@@ -41,11 +41,10 @@ export class ParameterInputComponent implements ControlValueAccessor, OnChanges,
         this._subs.push(this.parameterValue.valueChanges.pipe(
             distinctUntilChanged(),
         ).subscribe((query: string) => {
-                if (this._propagateChange) {
-                    this._propagateChange(query);
-                }
-            }),
-        );
+            if (this._propagateChange) {
+                this._propagateChange(query);
+            }
+        }));
     }
 
     public ngOnChanges(changes) {

desktop/src/app/components/gallery/submit/submit-ncj-template.component.ts

@@ -347,7 +347,7 @@ export class SubmitNcjTemplateComponent implements OnInit, OnChanges, OnDestroy
             let validator = Validators.required;
             if (exists(template.parameters[key].defaultValue)) {
                 defaultValue = String(template.parameters[key].defaultValue);
-                if (template.parameters[key].defaultValue === "") {
+                if (defaultValue.trim() === "") {
                     validator = null;
                 }
             }

desktop/src/app/components/pool/action/add/container-configuration-picker/container-configuration-picker.component.spec.ts

@@ -6,7 +6,7 @@ import { I18nTestingModule } from "@batch-flask/core/testing";
 import { SelectComponent, SelectModule } from "@batch-flask/ui";
 import { FormModule } from "@batch-flask/ui/form";
 import { ContainerConfigurationAttributes, ContainerType } from "app/models";
-import { ContainerConfigurationDto } from "app/models/dtos";
+import { ContainerConfigurationDto, ContainerRegistryDto } from "app/models/dtos";
 import { ContainerConfigurationPickerComponent } from "./container-configuration-picker.component";
 import { ContainerImagesPickerComponent } from "./images-picker/container-images-picker.component";
 import { ContainerRegistryPickerComponent } from "./registry-picker/container-registry-picker.component";
@@ -97,7 +97,7 @@ describe("ContainerConfigurationPickerComponent", () => {
             type: ContainerType.DockerCompatible,
             containerImageNames: [],
             containerRegistries: [
-                { username: "foo", password: "pass123!", registryServer: "https://bar.com" },
+                { username: "foo", password: "pass123!", registryServer: "https://bar.com" } as ContainerRegistryDto,
             ],
         }));
     });

desktop/src/app/models/azure-batch/pool/pool.spec.ts

@@ -1,4 +1,4 @@
-import { ContainerType } from "app/models";
+import { ContainerRegistry, ContainerType } from "app/models";
 import { List } from "immutable";
 import { Pool } from "./pool";
 
@@ -13,7 +13,7 @@ describe("Pool Model", () => {
                             username: "abc",
                             password: "foo",
                             registryServer: "hub.docker.com",
-                        },
+                        } as ContainerRegistry,
                     ],
                 },
             },

desktop/src/app/models/blob-container.ts

@@ -8,14 +8,14 @@ export interface BlobContainerAttributes {
     name: string;
     publicAccessLevel: string;
     metadata?: any;
-    lastModified: Date;
+    lastModified?: Date;
     lease?: Partial<ContainerLeaseAttributes>;
 }
 
 /**
  * Class for displaying blob container information.
  */
-@Model()
+@Model("BlobContainer")
 export class BlobContainer extends Record<BlobContainerAttributes> implements NavigableRecord {
     // container name
     @Prop() public id: string;
@@ -25,7 +25,7 @@ export class BlobContainer extends Record<BlobContainerAttributes> implements Na
 
     @Prop() public publicAccessLevel: string;
     @Prop() public metadata: any;
-    @Prop() public lastModified: Date;
+    @Prop() public lastModified?: Date;
     @Prop() public lease: ContainerLease;
     @Prop() public storageAccountId: string;
 

desktop/src/app/services/core/data/storage-list-getter.ts

@@ -1,5 +1,6 @@
 import { Type } from "@angular/core";
 import { ContinuationToken, ListGetter, ListGetterConfig, Record, ServerError } from "@batch-flask/core";
+import { BlobStorageClientProxy } from "app/services/storage";
 import { StorageBlobResult } from "app/services/storage/models";
 import { StorageClientService } from "app/services/storage/storage-client.service";
 import { Observable, from, throwError } from "rxjs";
@@ -12,13 +13,15 @@ export interface StorageBaseParams {
 export interface StorageListConfig<TEntity extends Record<any>, TParams extends StorageBaseParams>
     extends ListGetterConfig<TEntity, TParams> {
 
-    getData: (client: any, params: TParams, options: any, token: any) => Promise<StorageBlobResult<TEntity[]>>;
+    getData: (client: BlobStorageClientProxy, params: TParams, options: any, token: any) =>
+        Promise<StorageBlobResult<TEntity[]>>;
 }
 
 export class StorageListGetter<TEntity extends Record<any>, TParams extends StorageBaseParams>
     extends ListGetter<TEntity, TParams> {
 
-    private _getData: (client: any, params: TParams, options: any, token: any) => Promise<StorageBlobResult<TEntity[]>>;
+    private _getData: (client: BlobStorageClientProxy, params: TParams, options: any, token: any) =>
+        Promise<StorageBlobResult<TEntity[]>>;
 
     constructor(
         type: Type<TEntity>,

desktop/src/app/services/storage/blob-storage-client-proxy.ts

@@ -4,6 +4,7 @@ import { IpcEvent } from "common/constants";
 import { SharedAccessPolicy } from "./models";
 import * as blob from "./models/storage-blob";
 import { BlobContentResult } from "./storage-blob.service";
+import { StorageClient } from "./storage-client.service";
 
 const storageIpc = IpcEvent.storageBlob;
 
@@ -18,7 +19,7 @@ export interface ListBlobResponse {
     };
 }
 
-export class BlobStorageClientProxy {
+export class BlobStorageClientProxy implements StorageClient {
 
     private storageInfo: { url: string; account: string, key: string };
 
@@ -172,8 +173,13 @@ export class BlobStorageClientProxy {
         containerName: string,
         options?: blob.RequestOptions
     ): Promise<blob.GetContainerPropertiesResult> {
-        return this.remote.send(storageIpc.getContainerProperties,
+        const result = await this.remote.send(storageIpc.getContainerProperties,
             { ...this.storageInfo, containerName, options });
+
+        // The container name is not returned by the API, but we add it here,
+        // since it's used by the UI model
+        result.data.name =  containerName;
+        return result;
     }
 
     /**