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

feat: #544-Add short.io as a shortening link #564

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat: #544-Add short.io as a shortening link #564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9c10d0f
short.io integration is in progress.
ak7550 Jan 19, 2025
4d9ef69
code implementation complete.
ak7550 Jan 19, 2025
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
91 changes: 91 additions & 0 deletions libraries/nestjs-libraries/src/short-linking/providers/short.io.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import { ShortLinking } from '@gitroom/nestjs-libraries/short-linking/short-linking.interface';


const options = {
headers: {
Authorization: `Bearer ${process.env.SHORT_IO_SECRET_KEY}`,
'Content-Type': 'application/json',
},
};
Comment on lines +6 to +9
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Handle missing SHORT_IO_SECRET_KEY environment variable gracefully

If process.env.SHORT_IO_SECRET_KEY is undefined, the Authorization header will be 'Bearer undefined', which may cause authentication failures or security issues when making API calls. It's important to check if the environment variable is set and handle the absence appropriately.

Apply this diff to ensure the secret key is present and throw an error if it's missing:

 const options = {
   headers: {
-    Authorization: `Bearer ${process.env.SHORT_IO_SECRET_KEY}`,
+    Authorization: `Bearer ${process.env.SHORT_IO_SECRET_KEY || ''}`,
     'Content-Type': 'application/json',
   },
 };
+if (!process.env.SHORT_IO_SECRET_KEY) {
+  throw new Error('SHORT_IO_SECRET_KEY environment variable is not set.');
+}

Alternatively, consider initializing the options within the class constructor after verifying the presence of the secret key.

📝 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
Authorization: `Bearer ${process.env.SHORT_IO_SECRET_KEY}`,
'Content-Type': 'application/json',
},
};
const options = {
headers: {
Authorization: `Bearer ${process.env.SHORT_IO_SECRET_KEY || ''}`,
'Content-Type': 'application/json',
},
};
if (!process.env.SHORT_IO_SECRET_KEY) {
throw new Error('SHORT_IO_SECRET_KEY environment variable is not set.');
}


export class ShortIo implements ShortLinking {
shortLinkDomain = 'short.io';

async linksStatistics ( links: string[] ) {
return Promise.all(
links.map(async (link) => {
const url = `https://api.short.io/links/expand?domain=${this.shortLinkDomain}&path=${link.split('/').pop()}`;
const response = await ( fetch( url, options )
.then( res => res.json() ) );

const linkStatisticsUrl = `https://statistics.short.io/statistics/link/${response.id}?period=last30&tz=UTC`;

const statResponse = await ( fetch( linkStatisticsUrl, options ).then( ( res ) => res.json() ) );

return {
short: response.shortURL,
original: response.originalURL,
clicks: statResponse.totalClicks,
};
})
);
}
Comment on lines +17 to +32
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling for API requests in linksStatistics method

The linksStatistics method lacks error handling for network failures or unexpected responses, which can lead to unhandled exceptions. It's important to catch errors from fetch and handle HTTP response statuses appropriately.

Apply this diff to include error handling:

       links.map(async (link) => {
         const url = `https://api.short.io/links/expand?domain=${this.shortLinkDomain}&path=${link.split('/').pop()}`;
-        const response = await ( fetch( url, options )
-          .then( res => res.json() ) );
+        let response;
+        try {
+          const res = await fetch(url, options);
+          if (!res.ok) {
+            throw new Error(`Failed to expand link: ${res.status} ${res.statusText}`);
+          }
+          response = await res.json();
+        } catch (error) {
+          console.error('Error fetching link expansion:', error);
+          throw error;
+        }

         const linkStatisticsUrl = `https://statistics.short.io/statistics/link/${response.id}?period=last30&tz=UTC`;

-        const statResponse = await ( fetch( linkStatisticsUrl, options ).then( ( res ) => res.json() ) );
+        let statResponse;
+        try {
+          const res = await fetch(linkStatisticsUrl, options);
+          if (!res.ok) {
+            throw new Error(`Failed to get link statistics: ${res.status} ${res.statusText}`);
+          }
+          statResponse = await res.json();
+        } catch (error) {
+          console.error('Error fetching link statistics:', error);
+          throw error;
+        }

         return {
           short: response.shortURL,
           original: response.originalURL,
           clicks: statResponse.totalClicks,
         };
       })

This ensures that any errors during the fetch operations are caught and can be handled appropriately by the calling code.

📝 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 = `https://api.short.io/links/expand?domain=${this.shortLinkDomain}&path=${link.split('/').pop()}`;
const response = await ( fetch( url, options )
.then( res => res.json() ) );
const linkStatisticsUrl = `https://statistics.short.io/statistics/link/${response.id}?period=last30&tz=UTC`;
const statResponse = await ( fetch( linkStatisticsUrl, options ).then( ( res ) => res.json() ) );
return {
short: response.shortURL,
original: response.originalURL,
clicks: statResponse.totalClicks,
};
})
);
}
const url = `https://api.short.io/links/expand?domain=${this.shortLinkDomain}&path=${link.split('/').pop()}`;
let response;
try {
const res = await fetch(url, options);
if (!res.ok) {
throw new Error(`Failed to expand link: ${res.status} ${res.statusText}`);
}
response = await res.json();
} catch (error) {
console.error('Error fetching link expansion:', error);
throw error;
}
const linkStatisticsUrl = `https://statistics.short.io/statistics/link/${response.id}?period=last30&tz=UTC`;
let statResponse;
try {
const res = await fetch(linkStatisticsUrl, options);
if (!res.ok) {
throw new Error(`Failed to get link statistics: ${res.status} ${res.statusText}`);
}
statResponse = await res.json();
} catch (error) {
console.error('Error fetching link statistics:', error);
throw error;
}
return {
short: response.shortURL,
original: response.originalURL,
clicks: statResponse.totalClicks,
};
})
);
}


async convertLinkToShortLink ( id: string, link: string ) {
const response = await fetch( `https://api.short.io/links`, {
...options,
method: 'POST',
body: JSON.stringify( {
url: link,
tenantId: id,
domain: this.shortLinkDomain,
originalURL: link

} ),
} ).then( ( res ) => res.json() );

return response.shortURL;
}
Comment on lines +34 to +48
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix API parameters and add error handling in convertLinkToShortLink

The method has duplicate parameters and lacks error handling.

Apply this diff:

-  async convertLinkToShortLink(id: string, link: string) {
+  async convertLinkToShortLink(id: string, link: string): Promise<string> {
+    if (!link) {
+      throw new Error('Link is required');
+    }
+
+    try {
       const response = await fetch(`https://api.short.io/links`, {
         ...options,
         method: 'POST',
         body: JSON.stringify({
-          url: link,
-          tenantId: id,
+          originalURL: link,
+          domain_id: id,
           domain: this.shortLinkDomain,
-          originalURL: link
-
         }),
-      }).then((res) => res.json());
+      });
+
+      if (!response.ok) {
+        throw new Error(`Failed to create short link: ${response.status} ${response.statusText}`);
+      }
+
+      const data = await response.json();
+      return data.shortURL;
+    } catch (error) {
+      console.error('Error creating short link:', error);
+      throw error;
+    }
-    return response.shortURL;
   }
📝 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
async convertLinkToShortLink ( id: string, link: string ) {
const response = await fetch( `https://api.short.io/links`, {
...options,
method: 'POST',
body: JSON.stringify( {
url: link,
tenantId: id,
domain: this.shortLinkDomain,
originalURL: link
} ),
} ).then( ( res ) => res.json() );
return response.shortURL;
}
async convertLinkToShortLink(id: string, link: string): Promise<string> {
if (!link) {
throw new Error('Link is required');
}
try {
const response = await fetch(`https://api.short.io/links`, {
...options,
method: 'POST',
body: JSON.stringify({
originalURL: link,
domain_id: id,
domain: this.shortLinkDomain,
}),
});
if (!response.ok) {
throw new Error(`Failed to create short link: ${response.status} ${response.statusText}`);
}
const data = await response.json();
return data.shortURL;
} catch (error) {
console.error('Error creating short link:', error);
throw error;
}
}


async convertShortLinkToLink(shortLink: string) {
return await(
await(
await fetch(
`https://api.short.io/links/expand?domain=${this.shortLinkDomain}&path=${shortLink.split('/').pop()}`,
options
)
).json()
).originalURL;
}
Comment on lines +50 to +59
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Simplify promise handling and add error handling in convertShortLinkToLink

The method has unnecessary nested awaits and lacks error handling.

Apply this diff:

-  async convertShortLinkToLink(shortLink: string) {
-    return await(
-      await(
-        await fetch(
-          `https://api.short.io/links/expand?domain=${this.shortLinkDomain}&path=${shortLink.split('/').pop()}`,
-          options
-        )
-      ).json()
-    ).originalURL;
+  async convertShortLinkToLink(shortLink: string): Promise<string> {
+    if (!shortLink) {
+      throw new Error('Short link is required');
+    }
+
+    try {
+      const path = shortLink.split('/').pop();
+      if (!path) {
+        throw new Error('Invalid short link format');
+      }
+
+      const response = await fetch(
+        `https://api.short.io/links/expand?domain=${this.shortLinkDomain}&path=${path}`,
+        this.options
+      );
+
+      if (!response.ok) {
+        throw new Error(`Failed to expand short link: ${response.status} ${response.statusText}`);
+      }
+
+      const data = await response.json();
+      return data.originalURL;
+    } catch (error) {
+      console.error('Error expanding short link:', error);
+      throw error;
+    }
   }

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


// recursive functions that gets maximum 100 links per request if there are less than 100 links stop the recursion
async getAllLinksStatistics(
id: string,
page = 1
): Promise<{ short: string; original: string; clicks: string }[]> {
const response = await (
await fetch(
`https://api.short.io/api/links?domain_id=${id}&limit=150`,
options
)
).json();

const mapLinks = response.links.map(async ( link: any ) => {
const linkStatisticsUrl = `https://statistics.short.io/statistics/link/${response.id}?period=last30&tz=UTC`;

const statResponse = await(fetch(linkStatisticsUrl, options).then((res) => res.json()));

return {
short: link,
original: response.url,
clicks: statResponse.totalClicks,
};
} );

if (mapLinks.length < 100) {
return mapLinks;
}

return [...mapLinks, ...(await this.getAllLinksStatistics(id, page + 1))];
}
}
5 changes: 5 additions & 0 deletions libraries/nestjs-libraries/src/short-linking/short.link.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,17 @@ import { Dub } from '@gitroom/nestjs-libraries/short-linking/providers/dub';
import { Empty } from '@gitroom/nestjs-libraries/short-linking/providers/empty';
import { ShortLinking } from '@gitroom/nestjs-libraries/short-linking/short-linking.interface';
import { Injectable } from '@nestjs/common';
import { ShortIo } from './providers/short.io';

const getProvider = (): ShortLinking => {
if (process.env.DUB_TOKEN) {
return new Dub();
}

if ( process.env.SHORT_IO_SECRET_KEY ) {
return new ShortIo();
}

return new Empty();
};

Expand Down