Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/13-pyrobird-cli-flask' into 14-s…
Browse files Browse the repository at this point in the history
…cene-introspection-sidebar
  • Loading branch information
sqrd-max committed Oct 15, 2024
2 parents 5e3cb8a + ab4d083 commit 49980f3
Show file tree
Hide file tree
Showing 11 changed files with 63 additions and 126 deletions.
12 changes: 2 additions & 10 deletions firebird-ng/src/app/pages/input-config/input-config.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -147,14 +147,6 @@ <h6 class="mb-0">API URL:</h6>
</tr>
</thead>
<tbody>
<tr>
<td>Server Port</td>
<td>{{ firebirdConfig.serverPort }}</td>
</tr>
<tr>
<td>Server Host</td>
<td>{{ firebirdConfig.serverHost }}</td>
</tr>
<tr>
<td>Served by Pyrobird</td>
<td>{{ firebirdConfig.servedByPyrobird ? 'Yes' : 'No' }}</td>
Expand All @@ -164,8 +156,8 @@ <h6 class="mb-0">API URL:</h6>
<td>{{ firebirdConfig.apiAvailable ? 'Yes' : 'No' }}</td>
</tr>
<tr>
<td>Use Authentication</td>
<td>{{ firebirdConfig.useAuthentication ? 'Yes' : 'No' }}</td>
<td>API Base URL</td>
<td>{{ firebirdConfig.apiBaseUrl }}</td>
</tr>
<tr>
<td>Log Level</td>
Expand Down
39 changes: 6 additions & 33 deletions firebird-ng/src/app/services/server-config.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,46 +21,19 @@ describe('FirebirdConfigService', () => {
});
//
it('should fetch and parse JSONC data correctly', () => {
const dummyConfig = { serverPort: 5001 };
const dummyConfig = { apiBaseUrl: "http://localhost:5454" };
const jsoncData = '{ "key": "value" // comment }';

service.loadConfig().then(()=>{
expect(service.config.serverPort).toBe(5001);
});

// Set up the HttpTestingController
const req = httpMock.expectOne(service['configUrl']);
expect(req.request.method).toBe('GET');
req.flush('{ "serverPort": 5001, "useAuthentication": true, "logLevel": "info" }'); // Mock the HTTP response
req.flush('{ "apiBaseUrl": "http://localhost:5454", "logLevel": "info" }'); // Mock the HTTP response

service.loadConfig().then(()=>{
expect(service.config.apiBaseUrl).toBe("http://localhost:5454");
});

});
//
// it('should handle HTTP errors gracefully', () => {
// const errorResponse = new ErrorEvent('Network error');
//
// service.getConfig().subscribe({
// next: config => fail('should have failed with the network error'),
// error: error => expect(error).toBeTruthy(),
// complete: () => fail('The request should not complete successfully')
// });
//
// });
});


// import { TestBed } from '@angular/core/testing';
//
// import { ServerConfigService } from './firebird-config.service';
//
// describe('ServerConfigService', () => {
// let service: ServerConfigService;
//
// beforeEach(() => {
// TestBed.configureTestingModule({});
// service = TestBed.inject(ServerConfigService);
// });
//
// it('should be created', () => {
// expect(service).toBeTruthy();
// });
// });
8 changes: 2 additions & 6 deletions firebird-ng/src/app/services/server-config.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,16 @@ import {BehaviorSubject, Observable, catchError, map, of, firstValueFrom} from "


export interface ServerConfig {
serverPort: number;
serverHost: string;
servedByPyrobird: boolean;
apiAvailable: boolean;
useAuthentication: boolean;
apiBaseUrl: string;
logLevel: string;
}

export const defaultFirebirdConfig: ServerConfig = {
serverPort: 5454,
serverHost: "localhost",
apiAvailable: false,
apiBaseUrl: "",
servedByPyrobird: false,
useAuthentication: true,
logLevel: 'info'
};

Expand Down
5 changes: 3 additions & 2 deletions firebird-ng/src/app/services/url.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ describe('UrlService', () => {
serverConfigService = {
config: {
servedByPyrobird: false,
serverHost: 'localhost',
serverPort: 5000
apiAvailable: true,
apiBaseUrl: 'http://localhost:5454',
}
};

Expand All @@ -58,6 +58,7 @@ describe('UrlService', () => {
const expectedUrl = 'http://localhost:5454/api/v1/download?f=%2Fpath%2Fto%2Ffile.root';

const resolvedUrl = service.resolveDownloadUrl(inputUrl);

expect(resolvedUrl).toBe(expectedUrl);
}));

Expand Down
6 changes: 2 additions & 4 deletions firebird-ng/src/app/services/url.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,14 @@ import { ServerConfigService } from "./server-config.service";
* - Standalone static site without backend API.
* - Served by a Flask application (e.g., PyroBird) with a backend API.
* - Served by another service with or without a backend API.
* - Users may configure a custom API endpoint via `UserConfigService`.
* - The service needs to resolve URLs for files that may:
* - Be accessible over HTTP/HTTPS.
* - Be local files requiring backend API to serve them.
* - Use custom protocols (e.g., `asset://`, `epic://`).
*
* ### Use Cases:
* - **Case 1: Downloading Files**
* - **1.1**: Input URL starts with `http://` or `https://`. The URL is used as is.
* - **1.1**: Input URL starts with protocol like `root://`, `http://` or `https://`. The URL is used as is.
* - **1.2**: Input URL has no protocol or is a local file. The service checks if a backend is available and constructs the download endpoint URL.
* - **Case 2: Converting Files**
* - **2.1 & 2.2**: Input URL is converted using the backend 'convert' endpoint, regardless of the original protocol.
Expand Down Expand Up @@ -96,8 +95,7 @@ export class UrlService {
this.isBackendAvailable = servedByPyrobird || userUseApi;

if (servedByPyrobird) {
const protocol = window.location.protocol; // 'http:' or 'https:'
this.serverAddress = `${protocol}//${this.serverConfigService.config.serverHost}:${this.serverConfigService.config.serverPort}`;
this.serverAddress = this.serverConfigService.config.apiBaseUrl;
} else if (userUseApi) {
this.serverAddress = userServerUrl;
} else {
Expand Down
46 changes: 1 addition & 45 deletions firebird-ng/src/app/services/user-config.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ describe('UrlService', () => {
const mockServerConfigService = {
config: {
servedByPyrobird: false,
serverHost: 'localhost',
serverPort: 5000
apiBaseUrl: 'http:/localhost:4545'
}
};

Expand Down Expand Up @@ -162,47 +161,4 @@ describe('UrlService', () => {
expect(resolvedUrl).toBe(expectedUrl);
});
});

describe('backend availability and server address', () => {
it('should use PyroBird server address if servedByPyrobird is true', () => {
// Simulate PyroBird server
serverConfigService.config.servedByPyrobird = true;
serverConfigService.config.serverHost = 'pyrobirdhost';
serverConfigService.config.serverPort = 8080;

const expectedServerAddress = `${window.location.protocol}//pyrobirdhost:8080`;

// Reinitialize the service to pick up new config
service = new UrlService(userConfigService, serverConfigService);

// Access private property via getter (assuming you add getters)
// expect(service.getServerAddress()).toBe(expectedServerAddress);

// Or access private property directly for testing purposes
expect((service as any).serverAddress).toBe(expectedServerAddress);
});

it('should use user-configured server address if localServerUseApi is true', () => {
userConfigService.localServerUseApi.subject.next(true);
userConfigService.localServerUrl.subject.next('http://customserver:1234');

const expectedServerAddress = 'http://customserver:1234';

// Reinitialize the service to pick up new config
service = new UrlService(userConfigService, serverConfigService);

expect((service as any).serverAddress).toBe(expectedServerAddress);
});

it('should have backend unavailable when neither PyroBird nor user API is configured', () => {
userConfigService.localServerUseApi.subject.next(false);
serverConfigService.config.servedByPyrobird = false;

// Reinitialize the service to pick up new config
service = new UrlService(userConfigService, serverConfigService);

expect((service as any).isBackendAvailable).toBe(false);
expect((service as any).serverAddress).toBe('');
});
});
});
4 changes: 2 additions & 2 deletions pyrobird/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,9 @@ This is technical explanation of what is under the hood of the server part

### Configuration Options
- **DOWNLOAD_PATH**: `str[getcwd()]`, Specifies the directory from which files can be downloaded when using relative paths.
- **DOWNLOAD_DISABLE**: `bool[False]` If set to `True`, all download functionalities are disabled.
- **DOWNLOAD_IS_DISABLED**: `bool[False]` If set to `True`, all download functionalities are disabled.
- **DOWNLOAD_IS_UNRESTRICTED**: `bool[False]`, allows unrestricted access to download any file, including sensitive ones.
- **DOWNLOAD_ALLOW_CORS**: `bool[False]`, If set to `True`, enables Cross-Origin Resource Sharing (CORS) for download routes.
- **CORS_IS_ALLOWED**: `bool[False]`, If set to `True`, enables Cross-Origin Resource Sharing (CORS) for download routes.



Expand Down
4 changes: 2 additions & 2 deletions pyrobird/example_wsgi.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
config = {
"DOWNLOAD_DISABLE": True,
"DOWNLOAD_IS_DISABLED": True,
"DOWNLOAD_IS_UNRESTRICTED": False,
"DOWNLOAD_ALLOW_CORS": True
"CORS_IS_ALLOWED": True
}

from pyrobird.server import flask_app as application
Expand Down
16 changes: 11 additions & 5 deletions pyrobird/src/pyrobird/cli/serve.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import logging
import click
import pyrobird.server
from pyrobird.server import CFG_DOWNLOAD_IS_UNRESTRICTED, CFG_DOWNLOAD_IS_DISABLED, CFG_DOWNLOAD_PATH, \
CFG_CORS_IS_ALLOWED, CFG_API_BASE_URL, CFG_FIREBIRD_CONFIG_PATH

# Configure logging
logger = logging.getLogger(__name__)
Expand All @@ -26,8 +28,10 @@
@click.option("--work-path", "work_path", show_default=True, default="", help="Set the base directory path for file downloads. Defaults to the current working directory.")
@click.option("--host", "host", default="", help="Set the host for development server to listen to")
@click.option("--port", "port", default="", help="Set the port for development server to listen to")
@click.option("--api-url", "api_url", default="", help="Force to use this address as backend API base URL. E.g. https://my-server:1234/")
@click.option("--config", "config_path", default="", help="Path to firebird config.jsonc if used a custom")
@click.pass_context
def serve(ctx, unsecure_files, allow_cors, disable_download, work_path, host, port):
def serve(ctx, unsecure_files, allow_cors, disable_download, work_path, host, port, api_url, config_path):
"""
Start the server that serves Firebird frontend and can communicate with it.
Expand Down Expand Up @@ -61,10 +65,12 @@ def serve(ctx, unsecure_files, allow_cors, disable_download, work_path, host, po
port = 5454

pyrobird.server.run(debug=True, host=host, port=port, config={
"DOWNLOAD_IS_UNRESTRICTED": unsecure_files,
"DOWNLOAD_DISABLE": disable_download,
"DOWNLOAD_PATH": work_path,
"DOWNLOAD_ALLOW_CORS": allow_cors})
CFG_DOWNLOAD_IS_UNRESTRICTED: unsecure_files,
CFG_DOWNLOAD_IS_DISABLED: disable_download,
CFG_DOWNLOAD_PATH: work_path,
CFG_CORS_IS_ALLOWED: allow_cors,
CFG_API_BASE_URL: api_url,
CFG_FIREBIRD_CONFIG_PATH: config_path})


if __name__ == '__main__':
Expand Down
39 changes: 27 additions & 12 deletions pyrobird/src/pyrobird/server/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,21 @@
flask_app.config.update()

# Compression config
# We want to use compression only transfering JSON for now...
# We want to use compression only transferring JSON for now...
flask_app.config["COMPRESS_REGISTER"] = False # disable default compression of all requests
compress = Compress()
compress.init_app(flask_app)

# Config KEYS

CFG_DOWNLOAD_IS_UNRESTRICTED = "DOWNLOAD_IS_UNRESTRICTED"
CFG_DOWNLOAD_IS_DISABLED = "DOWNLOAD_IS_DISABLED"
CFG_DOWNLOAD_PATH = "DOWNLOAD_PATH"
CFG_CORS_IS_ALLOWED = "DOWNLOAD_ALLOW_CORS"
CFG_API_BASE_URL = "API_BASE_URL"
CFG_FIREBIRD_CONFIG_PATH = "FIREBIRD_CONFIG_PATH"



class ExcludeAPIConverter(BaseConverter):
"""
Expand Down Expand Up @@ -80,7 +90,7 @@ def _can_user_download_file(filename):
- bool: True if the file can be downloaded, False otherwise.
Process:
- If downloading is globally disabled (DOWNLOAD_DISABLE=True), returns False.
- If downloading is globally disabled (DOWNLOAD_IS_DISABLED=True), returns False.
- If unrestricted downloads are allowed (DOWNLOAD_IS_UNRESTRICTED=True), returns True.
- For relative paths, assumes that the download is allowable.
- For absolute paths, checks that the file resides within the configured DOWNLOAD_PATH.
Expand All @@ -90,12 +100,12 @@ def _can_user_download_file(filename):
app = flask.current_app

# If any downloads are disabled
if app.config.get("DOWNLOAD_DISABLE") is True:
logger.warning("Can't download file. DOWNLOAD_DISABLE=True")
if app.config.get(CFG_DOWNLOAD_IS_DISABLED) is True:
logger.warning("Can't download file. DOWNLOAD_IS_DISABLED=True")
return False

# If we allow any download
unrestricted_download = app.config.get("DOWNLOAD_IS_UNRESTRICTED") is True
unrestricted_download = app.config.get(CFG_DOWNLOAD_IS_UNRESTRICTED) is True
if unrestricted_download:
return True

Expand All @@ -106,7 +116,7 @@ def _can_user_download_file(filename):

# HERE we have and absolute path! Check if it is safe to download

allowed_path = app.config.get("DOWNLOAD_PATH")
allowed_path = app.config.get(CFG_DOWNLOAD_PATH)
if not allowed_path:
allowed_path = os.getcwd()

Expand Down Expand Up @@ -138,7 +148,7 @@ def download_file(filename=None):

# If it is relative, combine it with DOWNLOAD_PATH
if not os.path.isabs(filename):
download_path = flask.current_app.config.get("DOWNLOAD_PATH")
download_path = flask.current_app.config.get(CFG_DOWNLOAD_PATH)
if not download_path:
download_path = os.getcwd()

Expand Down Expand Up @@ -202,7 +212,7 @@ def open_edm4eic_file(filename=None, file_type="edm4eic", entries="0"):
if not is_remote:
# If it is relative, combine it with DOWNLOAD_PATH
if not os.path.isabs(filename):
download_path = flask.current_app.config.get("DOWNLOAD_PATH")
download_path = flask.current_app.config.get(CFG_DOWNLOAD_PATH)
if not download_path:
download_path = os.getcwd()

Expand Down Expand Up @@ -272,7 +282,9 @@ def asset_config():
It reads out existing file adding server info
"""
config_path = 'assets/config.jsonc'
config_path = flask_app.config.get(CFG_FIREBIRD_CONFIG_PATH)
if not config_path:
config_path = 'assets/config.jsonc'
config_dict = {}

os_config_path = os.path.join(flask_app.static_folder, 'assets', 'config.jsonc')
Expand All @@ -286,7 +298,7 @@ def asset_config():
except Exception as ex:
logger.error(f"error opening {config_path}: {ex}")

host = 'unknown'
host = 'localhost'
port = 80

tokens = request.host.split(':')
Expand All @@ -309,6 +321,8 @@ def asset_config():
config_dict['serverHost'] = host
config_dict['servedByPyrobird'] = True
config_dict['apiAvailable'] = True
config_api_url = flask_app.config.get(CFG_API_BASE_URL)
config_dict['apiBaseUrl'] = config_api_url if config_api_url else f"{request.scheme}://{host}:{port}"

# Convert the updated dictionary to JSON
return jsonify(config_dict)
Expand Down Expand Up @@ -344,18 +358,19 @@ def shutdown():
def run(config=None, host=None, port=5454, debug=False, load_dotenv=False):
"""Runs flask server"""
if config:
if isinstance(config, Config) or isinstance(config, map):
if isinstance(config, flask.Config) or isinstance(config, map):
flask_app.config.from_mapping(config)
else:
flask_app.config.from_object(config)

if flask_app.config and flask_app.config.get("DOWNLOAD_ALLOW_CORS") is True:
if flask_app.config and flask_app.config.get(CFG_CORS_IS_ALLOWED) is True:
from flask_cors import CORS

# Enable CORS for all routes and specify the domains and settings
CORS(flask_app, resources={
r"/download/*": {"origins": "*"},
r"/api/v1/*": {"origins": "*"},
r"/assets/config.jsonc": {"origins": "*"},
})

logger.debug("Serve path:")
Expand Down
Loading

0 comments on commit 49980f3

Please sign in to comment.