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

Include site category data in Readings schema #3884

Merged
merged 2 commits into from
Nov 24, 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
24 changes: 24 additions & 0 deletions src/device-registry/models/Reading.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,29 @@ const HealthTipsSchema = new Schema(
{ _id: false }
);

const categorySchema = new Schema(
{
area_name: { type: String },
category: { type: String },
highway: { type: String },
landuse: { type: String },
latitude: { type: Number },
longitude: { type: Number },
natural: { type: String },
search_radius: { type: Number },
waterway: { type: String },
tags: [
{
type: String,
trim: true,
},
],
},
{
_id: false,
}
);

const SiteDetailsSchema = new Schema(
{
_id: Schema.Types.ObjectId,
Expand All @@ -42,6 +65,7 @@ const SiteDetailsSchema = new Schema(
bearing_in_radians: Number,
description: String,
data_provider: String,
site_category: { type: categorySchema },
},
{ _id: false }
);
Expand Down
149 changes: 149 additions & 0 deletions src/device-registry/utils/scripts/api-response-transformer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
/**
* Transforms API response data into the required format for the second API
* @param {Object} inputData - The response from the first API
* @returns {Object} Formatted data for the second API
*
*/
const axios = require("axios");
const isEmpty = require("is-empty");

function transformApiResponse(inputData) {
// Extract site-category data for direct mapping
const siteCategory = inputData.site["site-category"];

// Initialize the base structure with direct mappings
const result = {
area_name: siteCategory.area_name,
category: siteCategory.category,
highway: siteCategory.highway,
landuse: siteCategory.landuse,
latitude: siteCategory.latitude,
longitude: siteCategory.longitude,
natural: siteCategory.natural,
search_radius: siteCategory.search_radius,
waterway: siteCategory.waterway,
tags: [],
};

// Process OSM_info array to extract unique values
const osmInfoArray = inputData.site.OSM_info;
const uniqueValues = new Map();

// Helper function to clean and extract value from OSM info line
const extractValue = (line) => {
const [key, value] = line.trim().split(": ");
return { key: key.trim(), value: value.trim() };
};

// Process each entry in OSM_info
for (let i = 0; i < osmInfoArray.length; i++) {
const line = osmInfoArray[i];

// Skip the "Found OSM data:" lines
if (line.includes("Found OSM data:")) continue;

const { key, value } = extractValue(line);

// Skip empty or "unknown" values
if (!value || value === "unknown") continue;

// For Location, format it as a string
if (key === "Location") {
const locationStr = value.replace(/[()]/g, "");
uniqueValues.set(key, locationStr);
}
// For other keys, store if not already present or if current value is more specific
else if (
!uniqueValues.has(key) ||
(uniqueValues.get(key) === "unknown" && value !== "unknown")
) {
uniqueValues.set(key, value);
}
}

// Convert unique values to tags array
uniqueValues.forEach((value, key) => {
// Format the tag as "key: value"
const tag = `${key}: ${value}`;
result.tags.push(tag);
});

return result;
}

// Example usage:
const createRequestBody = (apiResponse) => {
try {
return transformApiResponse(apiResponse);
} catch (error) {
console.error("Error transforming API response:", error);
throw error;
}
};

// Make a GET request
const url = "http://localhost:3000/api/v2/devices/sites/summary";
const config = {
headers: {
Authorization: "",
},
Comment on lines +85 to +89
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use environment variables for configuration to enhance security and flexibility.

Hardcoding URLs and the Authorization header can lead to security risks and make the code less adaptable across different environments. Using environment variables or a configuration file allows for better security practices and easier configuration management.

};
axios
.get(url, config)
.then((response) => {
console.log("GET response site name" + ": ");
console.dir(response.data);
for (let i = 0; i < response.data.sites.length; i += 10) {
const batch = response.data.sites.slice(i, i + 10);
// Process batch of 10 items
batch.forEach(async (site) => {
// console.log("the site _id", site._id);
const url = `http://localhost:3000/api/v2/spatial/categorize_site?latitude=${site.latitude}&longitude=${site.longitude}`;

//***MAKE THE GET REQUEST */
axios
.get(url, config)
.then((response) => {
// console.log("PUT response:", response.data);
const apiResponse = response.data;
const requestBody = createRequestBody(apiResponse);
// console.log("requestBody", requestBody);
const updateURL = `http://localhost:3000/api/v2/devices/sites?id=${site._id}`;
const data = {
site_category: requestBody,
};

// Make a PUT request
axios
.put(updateURL, data, config)
.then((response) => {
console.log("PUT response:", response.data);
})
.catch((error) => {
if (error.response) {
console.log(error.response.status);
console.log(error.response.data);
} else {
console.log(error.message);
}
});
})
.catch((error) => {
if (error.response) {
console.log(error.response.status);
console.log(error.response.data);
} else {
console.log(error.message);
}
});
});
Comment on lines +96 to +139
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Refactor asynchronous code to use async/await consistently for improved readability.

In the batch processing loop, you're mixing async functions with .then() and .catch() promise handling. For better readability and error handling, consider refactoring the code to use async/await throughout.

Here's how you could refactor the code:

 // Process batch of items
-batch.forEach(async (site) => {
+for (const site of batch) {
+  try {
+    const url = `http://localhost:3000/api/v2/spatial/categorize_site?latitude=${site.latitude}&longitude=${site.longitude}`;
+
+    // Make the GET request
+    const response = await axios.get(url, config);
+    const apiResponse = response.data;
+    const requestBody = createRequestBody(apiResponse);
+
+    const updateURL = `http://localhost:3000/api/v2/devices/sites?id=${site._id}`;
+    const data = {
+      site_category: requestBody,
+    };
+
+    // Make the PUT request
+    const putResponse = await axios.put(updateURL, data, config);
+    console.log('PUT response:', putResponse.data);
+  } catch (error) {
+    if (error.response) {
+      console.log(error.response.status);
+      console.log(error.response.data);
+    } else {
+      console.log(error.message);
+    }
+  }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for (let i = 0; i < response.data.sites.length; i += 10) {
const batch = response.data.sites.slice(i, i + 10);
// Process batch of 10 items
batch.forEach(async (site) => {
// console.log("the site _id", site._id);
const url = `http://localhost:3000/api/v2/spatial/categorize_site?latitude=${site.latitude}&longitude=${site.longitude}`;
//***MAKE THE GET REQUEST */
axios
.get(url, config)
.then((response) => {
// console.log("PUT response:", response.data);
const apiResponse = response.data;
const requestBody = createRequestBody(apiResponse);
// console.log("requestBody", requestBody);
const updateURL = `http://localhost:3000/api/v2/devices/sites?id=${site._id}`;
const data = {
site_category: requestBody,
};
// Make a PUT request
axios
.put(updateURL, data, config)
.then((response) => {
console.log("PUT response:", response.data);
})
.catch((error) => {
if (error.response) {
console.log(error.response.status);
console.log(error.response.data);
} else {
console.log(error.message);
}
});
})
.catch((error) => {
if (error.response) {
console.log(error.response.status);
console.log(error.response.data);
} else {
console.log(error.message);
}
});
});
for (let i = 0; i < response.data.sites.length; i += 10) {
const batch = response.data.sites.slice(i, i + 10);
// Process batch of items
for (const site of batch) {
try {
const url = `http://localhost:3000/api/v2/spatial/categorize_site?latitude=${site.latitude}&longitude=${site.longitude}`;
// Make the GET request
const response = await axios.get(url, config);
const apiResponse = response.data;
const requestBody = createRequestBody(apiResponse);
const updateURL = `http://localhost:3000/api/v2/devices/sites?id=${site._id}`;
const data = {
site_category: requestBody,
};
// Make the PUT request
const putResponse = await axios.put(updateURL, data, config);
console.log('PUT response:', putResponse.data);
} catch (error) {
if (error.response) {
console.log(error.response.status);
console.log(error.response.data);
} else {
console.log(error.message);
}
}
}

}
})
.catch((error) => {
console.error("GET error:", error);
});

module.exports = {
createRequestBody,
transformApiResponse,
};
20 changes: 17 additions & 3 deletions src/device-registry/utils/scripts/bulk-script.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,24 @@
const axios = require("axios");
const isEmpty = require("is-empty");

function extractBeforeV2IncludingSlash(url) {
const v2Index = url.indexOf("v2/");

if (v2Index !== -1) {
// Found "v2/" in the string
return url.substring(0, v2Index + 3); // Include "v2/"
} else {
// "v2/" not found, return the original string
return url;
}
}
Comment on lines +4 to +14
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add input validation and edge case handling to the utility function

The function needs to be more robust to handle various edge cases and invalid inputs.

Consider this improved implementation:

 function extractBeforeV2IncludingSlash(url) {
+  if (!url || typeof url !== 'string') {
+    return '';
+  }
+
   const v2Index = url.indexOf("v2/");
 
   if (v2Index !== -1) {
-    // Found "v2/" in the string
     return url.substring(0, v2Index + 3); // Include "v2/"
   } else {
-    // "v2/" not found, return the original string
     return url;
   }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function extractBeforeV2IncludingSlash(url) {
const v2Index = url.indexOf("v2/");
if (v2Index !== -1) {
// Found "v2/" in the string
return url.substring(0, v2Index + 3); // Include "v2/"
} else {
// "v2/" not found, return the original string
return url;
}
}
function extractBeforeV2IncludingSlash(url) {
if (!url || typeof url !== 'string') {
return '';
}
const v2Index = url.indexOf("v2/");
if (v2Index !== -1) {
return url.substring(0, v2Index + 3); // Include "v2/"
} else {
return url;
}
}


/**
* Update each device to have the default network of VISIBLE_DEVICES_ONLY
*/

// Make a GET request
const url = "http://localhost:3000/api/v1/devices";
const url = "http://localhost:3000/api/v2/devices/summary";
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Use environment configuration for API endpoints

Hardcoding the localhost URL makes the script environment-dependent and could lead to issues in different deployment environments.

Consider using environment variables:

- const url = "http://localhost:3000/api/v2/devices/summary";
+ const BASE_URL = process.env.API_BASE_URL || "http://localhost:3000";
+ const url = `${BASE_URL}/api/v2/devices/summary`;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const url = "http://localhost:3000/api/v2/devices/summary";
const BASE_URL = process.env.API_BASE_URL || "http://localhost:3000";
const url = `${BASE_URL}/api/v2/devices/summary`;

💡 Codebase verification

Based on the results from our investigation, I can now provide a conclusive response about the API version migration.

Migration from v1 to v2 endpoints is incomplete

  • Found commented out v1 route in src/device-registry/bin/server.js: // app.use("/api/v1/devices", require("@routes/v1"));
  • The v1 routes directory still exists and contains active code
  • The v2 endpoint being used in bulk-script.js is correct, but the overall migration is not complete
🔗 Analysis chain

Verify complete migration from v1 to v2 endpoints

Let's ensure all v1 endpoints have been properly updated to v2 and that we're using all required v2 parameters.

Also applies to: 37-37


Let me gather more information about the device-registry endpoints to ensure a complete verification.

The previous results show v1 references in device-registry files, and we need to verify if there are any device-specific v1 endpoints still in use. This will help us confirm if the migration is complete and consistent.


Let me check the device-registry routes to verify the complete migration status.

This will help us verify if there are any v1 routes still in use and confirm the migration status of the endpoints.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining v1 API references
echo "Checking for v1 API references..."
rg -l "api/v1/" --type js

# Search for v2 endpoint usage patterns to verify consistency
echo "Checking v2 endpoint patterns..."
rg "api/v2/devices/(summary|soft)" --type js -A 2

Length of output: 3023


Script:

#!/bin/bash
# Check for any v1 device endpoints specifically
echo "Checking for v1 device endpoints..."
rg "api/v1/devices" --type js -A 2

# Check the device-registry config and routes for endpoint definitions
echo "Checking device-registry endpoint configurations..."
fd -e js -e ts . src/device-registry/routes --exec cat {}

Length of output: 102549


Script:

#!/bin/bash
# Check device-registry routes for v1 and v2 endpoint definitions
echo "Checking device-registry routes for API versions..."
fd -e js . src/device-registry/routes/v1 --exec cat {}
fd -e js . src/device-registry/routes/v2 --exec cat {}

# Check for any v1 device endpoints in use
echo "Checking for v1 device endpoint usage..."
rg "api/v1/devices" --type js -A 2 src/device-registry/

Length of output: 155385

const config = {
headers: {
Authorization: "",
Expand All @@ -22,9 +34,11 @@ axios
// Process batch of 10 items
batch.forEach(async (device) => {
// console.log("the device _id", device._id);
const url = `http://localhost:3000/api/v1/devices?id=${device._id}`;
const url = `http://localhost:3000/api/v2/devices/soft?id=${device._id}`;
const new_api_code = extractBeforeV2IncludingSlash(device.api_code);

const data = {
network: "airqo",
api_code: new_api_code,
Comment on lines +37 to +41
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve batch processing reliability and error handling

The current implementation has several potential issues:

  1. Using forEach with async operations doesn't guarantee sequential processing
  2. No rate limiting could overwhelm the API
  3. No progress tracking for large datasets

Consider this more robust approach:

- batch.forEach(async (device) => {
+ for (const device of batch) {
+   try {
      const url = `http://localhost:3000/api/v2/devices/soft?id=${device._id}`;
      const new_api_code = extractBeforeV2IncludingSlash(device.api_code);
+     if (!new_api_code) {
+       console.warn(`Skipping device ${device._id}: Invalid api_code`);
+       continue;
+     }
      const data = {
        api_code: new_api_code,
      };
+     await new Promise(resolve => setTimeout(resolve, 100)); // Rate limiting
+     const response = await axios.put(url, data, config);
+     console.log(`Updated device ${device._id}`);
+   } catch (error) {
+     console.error(`Failed to update device ${device._id}:`, error.message);
+   }
- });
+ }

Committable suggestion skipped: line range outside the PR's diff.

};

if (isEmpty(device.network)) {
Expand Down
Loading