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.

Sign up for GitHub

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

Jump to bottom

Improve filter #76

Merged
merged 11 commits into from
Oct 30, 2021
Merged

Improve filter #76

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b13e5bc
Merge pull request #38 from nemgrouplimited/dev
OlegMakarenko Feb 24, 2021
eff6506
Merge pull request #45 from nemgrouplimited/dev
AnthonyLaw Mar 13, 2021
8a7b731
Merge pull request #47 from nemgrouplimited/dev
AnthonyLaw Mar 14, 2021
4950816
Merge pull request #60 from nemgrouplimited/dev
OlegMakarenko May 25, 2021
f63adaf
Dev -> Main (#78)
AnthonyLaw Oct 30, 2021
2efae6d
Creating new version 1.1.1
Oct 30, 2021
b3e923e
Merge branch 'main' into dev
AnthonyLaw Oct 30, 2021
3048c1e
feat: added restGatewayUrl field
AnthonyLaw Oct 30, 2021
5823c4d
feat: add ssl node query
AnthonyLaw Oct 30, 2021
ac318a3
Merge branch 'dev' into improve-filter
AnthonyLaw Oct 30, 2021
d29c81c
fix: extra ":" string cause the query issue
AnthonyLaw Oct 30, 2021
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
2 changes: 1 addition & 1 deletion package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "symbol-statistics-service",
"version": "1.1.0",
"version": "1.1.1",
"description": "",
"scripts": {
"dev": "nodemon",
Expand Down
4 changes: 4 additions & 0 deletions src/models/Node.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ const NodeSchema: Schema = new Schema({
},
},
apiStatus: {
restGatewayUrl: {
type: String,
required: false,
},
isAvailable: {
type: Boolean,
required: false,
Expand Down
11 changes: 10 additions & 1 deletion src/routes/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ enum NodeFilter {
export class Routes {
static register = async (app: Express) => {
app.get('/nodes', async (req: Request, res: Response) => {
const { filter, limit } = req.query;
const { filter, limit, ssl } = req.query;

let searchCriteria: NodeSearchCriteria = {
filter: {
Expand All @@ -20,6 +20,15 @@ export class Routes {
limit: Number(limit) || 0,
};

// add ssl filter to query isHttpsEnabled nodes.
if (ssl) {
const isSSL = ssl.toString().toLocaleLowerCase() === 'true';
Copy link
Contributor

Choose a reason for hiding this comment

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

what if the ssl is undefined?

Copy link
Member Author

Choose a reason for hiding this comment

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

the list will return http and https together


Object.assign(searchCriteria.filter, {
'apiStatus.isHttpsEnabled': isSSL,
});
}

// Return error message filter is not support
if (filter && filter !== NodeFilter.Preferred && filter !== NodeFilter.Suggested) {
return UnsupportedFilterError.send(res, filter as string);
Expand Down
15 changes: 9 additions & 6 deletions src/services/ApiNodeService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ interface FinalizedBlock {
}

export interface ApiStatus {
restGatewayUrl: string;
isAvailable: boolean;
isHttpsEnabled?: boolean;
nodeStatus?: NodeStatus;
Expand Down Expand Up @@ -66,10 +67,10 @@ export class ApiNodeService {
static getStatus = async (host: string): Promise<ApiStatus> => {
try {
const isHttps = await ApiNodeService.isHttpsEnabled(host);
const protocol = isHttps ? 'https' : 'http';
const protocol = isHttps ? 'https:' : 'http:';
const port = isHttps ? 3001 : 3000;

logger.info(`Getting node status for: ${protocol}://${host}:${port}`);
logger.info(`Getting node status for: ${protocol}//${host}:${port}`);

const [nodeInfo, chainInfo, nodeServer, nodeHealth] = await Promise.all([
ApiNodeService.getNodeInfo(host, port, protocol),
Expand All @@ -79,6 +80,7 @@ export class ApiNodeService {
]);

let apiStatus = {
restGatewayUrl: `${protocol}//${host}:${port}`,
isAvailable: true,
lastStatusCheck: Date.now(),
};
Expand Down Expand Up @@ -118,6 +120,7 @@ export class ApiNodeService {
} catch (e) {
logger.error(`Fail to request host node status: ${host}`, e);
return {
restGatewayUrl: `http://${host}:3000`,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about this default value.

It could happen that the rest url is on 3001 https but rest fails to get the current status. This method may swap from 3001 to 3000 when 3000 might not be there

Copy link
Member Author

Choose a reason for hiding this comment

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

So if the node is not healthy or unreached, it nodes filter won't return it.
We can remove or change something unreached under the restGatewayUrl

Copy link
Contributor

Choose a reason for hiding this comment

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

if the catch is caught, why still return the json?

Copy link
Member Author

Choose a reason for hiding this comment

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

I just a record need to be stored in the database.

isAvailable: false,
lastStatusCheck: Date.now(),
};
Expand All @@ -126,7 +129,7 @@ export class ApiNodeService {

static getNodeInfo = async (host: string, port: number, protocol: string): Promise<NodeInfo | null> => {
try {
return (await HTTP.get(`${protocol}://${host}:${port}/node/info`)).data;
return (await HTTP.get(`${protocol}//${host}:${port}/node/info`)).data;
} catch (e) {
logger.error(`Fail to request /node/info: ${host}`, e);
return null;
Expand All @@ -135,7 +138,7 @@ export class ApiNodeService {

static getNodeChainInfo = async (host: string, port: number, protocol: string): Promise<ChainInfo | null> => {
try {
return (await HTTP.get(`${protocol}://${host}:${port}/chain/info`)).data;
return (await HTTP.get(`${protocol}//${host}:${port}/chain/info`)).data;
} catch (e) {
logger.error(`Fail to request /chain/info: ${host}`, e);
return null;
Expand All @@ -144,7 +147,7 @@ export class ApiNodeService {

static getNodeServer = async (host: string, port: number, protocol: string): Promise<ServerInfo | null> => {
try {
const nodeServerInfo = (await HTTP.get(`${protocol}://${host}:${port}/node/server`)).data;
const nodeServerInfo = (await HTTP.get(`${protocol}//${host}:${port}/node/server`)).data;

return nodeServerInfo.serverInfo;
} catch (e) {
Expand All @@ -155,7 +158,7 @@ export class ApiNodeService {

static getNodeHealth = async (host: string, port: number, protocol: string): Promise<NodeStatus | null> => {
try {
const health = (await HTTP.get(`${protocol}://${host}:${port}/node/health`)).data;
const health = (await HTTP.get(`${protocol}//${host}:${port}/node/health`)).data;

return health.status;
} catch (e) {
Expand Down