Skip to content

(fix) swagger stuff #8

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

(fix) swagger stuff #8

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
145 changes: 79 additions & 66 deletions config/initializers/couchbase.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,78 +6,91 @@
DB_CONN_STR = ENV['DB_CONN_STR']
DB_BUCKET_NAME = 'travel-sample' # Hardcoded bucket name

# Check if running in CI environment
if ENV['CI']
# Use environment variables from GitHub Secrets
options = Couchbase::Cluster::ClusterOptions.new
options.authenticate(DB_USERNAME, DB_PASSWORD)
COUCHBASE_CLUSTER = Couchbase::Cluster.connect(DB_CONN_STR, options)
else
# Load environment variables from dev.env file
require 'dotenv'
Dotenv.load('dev.env')
begin
# Check if running in CI environment
if ENV['CI']
# Use environment variables from GitHub Secrets
options = Couchbase::Cluster::ClusterOptions.new
options.authenticate(DB_USERNAME, DB_PASSWORD)
COUCHBASE_CLUSTER = Couchbase::Cluster.connect(DB_CONN_STR, options)
else
# Load environment variables from dev.env file
require 'dotenv'
Dotenv.load('dev.env')

# Define default values
DEFAULT_DB_USERNAME = 'Administrator'
DEFAULT_DB_PASSWORD = 'password'
DEFAULT_DB_CONN_STR = 'couchbase://localhost'
# Define default values
DEFAULT_DB_USERNAME = 'Administrator'
DEFAULT_DB_PASSWORD = 'password'
DEFAULT_DB_CONN_STR = 'couchbase://localhost'

# Get environment variables with fallback to default values
DB_USERNAME = ENV.fetch('DB_USERNAME', DEFAULT_DB_USERNAME)
DB_PASSWORD = ENV.fetch('DB_PASSWORD', DEFAULT_DB_PASSWORD)
DB_CONN_STR = ENV.fetch('DB_CONN_STR', DEFAULT_DB_CONN_STR)
# Get environment variables with fallback to default values
DB_USERNAME = ENV.fetch('DB_USERNAME', DEFAULT_DB_USERNAME)
DB_PASSWORD = ENV.fetch('DB_PASSWORD', DEFAULT_DB_PASSWORD)
DB_CONN_STR = ENV.fetch('DB_CONN_STR', DEFAULT_DB_CONN_STR)

# Connect to the Couchbase cluster
options = Couchbase::Cluster::ClusterOptions.new
options.authenticate(DB_USERNAME, DB_PASSWORD)
COUCHBASE_CLUSTER = Couchbase::Cluster.connect(DB_CONN_STR, options)
end
# Connect to the Couchbase cluster
options = Couchbase::Cluster::ClusterOptions.new
options.authenticate(DB_USERNAME, DB_PASSWORD)
COUCHBASE_CLUSTER = Couchbase::Cluster.connect(DB_CONN_STR, options)
Comment on lines +27 to +34
Copy link

Choose a reason for hiding this comment

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

high

Re-assigning constants DB_USERNAME, DB_PASSWORD, and DB_CONN_STR here is problematic. In Ruby, this generates a warning and is considered bad practice as it can lead to confusing behavior. These constants are already defined at the top of the file (lines 4-6).

A better approach is to use local variables to hold the configuration for the non-CI environment, and then use those to connect. This avoids constant re-assignment and makes the code's intent clearer.

    db_username = ENV.fetch('DB_USERNAME', DEFAULT_DB_USERNAME)
    db_password = ENV.fetch('DB_PASSWORD', DEFAULT_DB_PASSWORD)
    db_conn_str = ENV.fetch('DB_CONN_STR', DEFAULT_DB_CONN_STR)

    # Connect to the Couchbase cluster
    options = Couchbase::Cluster::ClusterOptions.new
    options.authenticate(db_username, db_password)
    COUCHBASE_CLUSTER = Couchbase::Cluster.connect(db_conn_str, options)

end

# Open the bucket
bucket = COUCHBASE_CLUSTER.bucket(DB_BUCKET_NAME)
# Open the bucket
bucket = COUCHBASE_CLUSTER.bucket(DB_BUCKET_NAME)

# Open the default collection
default_collection = bucket.default_collection
# Open the default collection
default_collection = bucket.default_collection

# Create scope and collections if they don't exist
begin
scope = bucket.scope('inventory')
rescue Couchbase::Error::ScopeNotFoundError
bucket.create_scope('inventory')
scope = bucket.scope('inventory')
end
# Create scope and collections if they don't exist
begin
scope = bucket.scope('inventory')
rescue Couchbase::Error::ScopeNotFoundError
bucket.create_scope('inventory')
scope = bucket.scope('inventory')
end

begin
# create hotel search index
index_file_path = 'hotel_search_index.json'
index_content = File.read(index_file_path)
index_data = JSON.parse(index_content)
name = index_data["name"]
index = Couchbase::Management::SearchIndex.new
index.name= index_data["name"]
index.type= index_data["type"]
index.uuid= index_data["uuid"] if index_data.has_key?("uuid")
index.params= index_data["params"] if index_data.has_key?("params")
index.source_name= index_data["sourceName"] if index_data.has_key?("sourceName")
index.source_type= index_data["sourceType"] if index_data.has_key?("sourceType")
index.source_uuid= index_data["sourceUUID"] if index_data.has_key?("sourceUUID")
index.source_params= index_data["sourceParams"] if index_data.has_key?("sourceParams")
index.plan_params= index_data["planParams"] if index_data.has_key?("planParams")
scope.search_indexes.upsert_index(index)
rescue StandardError => err
#puts err.full_message
end
begin
# create hotel search index
index_file_path = 'hotel_search_index.json'
index_content = File.read(index_file_path)
index_data = JSON.parse(index_content)
name = index_data["name"]
index = Couchbase::Management::SearchIndex.new
index.name= index_data["name"]
index.type= index_data["type"]
index.uuid= index_data["uuid"] if index_data.has_key?("uuid")
index.params= index_data["params"] if index_data.has_key?("params")
index.source_name= index_data["sourceName"] if index_data.has_key?("sourceName")
index.source_type= index_data["sourceType"] if index_data.has_key?("sourceType")
index.source_uuid= index_data["sourceUUID"] if index_data.has_key?("sourceUUID")
index.source_params= index_data["sourceParams"] if index_data.has_key?("sourceParams")
index.plan_params= index_data["planParams"] if index_data.has_key?("planParams")
Comment on lines +57 to +66
Copy link

Choose a reason for hiding this comment

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

medium

This block of code manually assigns properties from the parsed JSON to the SearchIndex object. This is verbose and hard to maintain. If the JSON structure in hotel_search_index.json changes, you'll need to update this code.

You can make this more dynamic and concise by iterating over the JSON data and using metaprogramming to call the appropriate setter methods on the index object. This will make the code more resilient to changes.

    index = Couchbase::Management::SearchIndex.new
    index_data.each do |key, value|
      setter = "#{key.underscore}="
      index.public_send(setter, value) if index.respond_to?(setter)
    end

scope.search_indexes.upsert_index(index)
rescue StandardError => err
#puts err.full_message
end

%w[airline airport route].each do |collection_name|
scope.collection(collection_name)
rescue Couchbase::Error::CollectionNotFoundError
scope.create_collection(collection_name)
end
%w[airline airport route].each do |collection_name|
scope.collection(collection_name)
rescue Couchbase::Error::CollectionNotFoundError
scope.create_collection(collection_name)
end

# Scope is declared as constant to run FTS queries
INVENTORY_SCOPE = scope
INDEX_NAME = name
AIRLINE_COLLECTION = INVENTORY_SCOPE.collection('airline')
AIRPORT_COLLECTION = INVENTORY_SCOPE.collection('airport')
ROUTE_COLLECTION = INVENTORY_SCOPE.collection('route')
HOTEL_COLLECTION = INVENTORY_SCOPE.collection('hotel')
# Scope is declared as constant to run FTS queries
INVENTORY_SCOPE = scope
INDEX_NAME = name
AIRLINE_COLLECTION = INVENTORY_SCOPE.collection('airline')
AIRPORT_COLLECTION = INVENTORY_SCOPE.collection('airport')
ROUTE_COLLECTION = INVENTORY_SCOPE.collection('route')
HOTEL_COLLECTION = INVENTORY_SCOPE.collection('hotel')
rescue StandardError => err
# Allow Rails to boot even if Couchbase is not reachable/misconfigured
warn_msg = "Couchbase initialization skipped: #{err.class}: #{err.message}"
defined?(Rails) ? Rails.logger.warn(warn_msg) : warn(warn_msg)
COUCHBASE_CLUSTER = nil
INVENTORY_SCOPE = nil
INDEX_NAME = nil
AIRLINE_COLLECTION = nil
AIRPORT_COLLECTION = nil
ROUTE_COLLECTION = nil
HOTEL_COLLECTION = nil
end
7 changes: 1 addition & 6 deletions spec/swagger_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,7 @@
paths: {},
servers: [
{
url: 'http://{defaultHost}',
variables: {
defaultHost: {
default: 'localhost:3000'
}
}
url: '/'
}
],
components: {
Expand Down
78 changes: 74 additions & 4 deletions swagger/v1/swagger.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,79 @@ paths:
type: string
'400':
description: bad request
"/api/v1/hotels/autocomplete":
get:
summary: Retrieve suggestion for Hotel names
Copy link

Choose a reason for hiding this comment

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

medium

The summary should use the plural form "suggestions" since the endpoint can return multiple hotel names.

      summary: Retrieve suggestions for Hotel names

tags:
- Hotels
parameters:
- name: name
in: query
description: name of the hotel
schema:
type: string
responses:
'200':
description: No suggestion
Copy link

Choose a reason for hiding this comment

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

medium

The description "No suggestion" for a successful response is misleading. It should describe the successful outcome, for example, that a list of suggestions is returned. The current description seems to be for a specific test case (an empty result) rather than the general behavior of the endpoint.

          description: A list of hotel name suggestions

content:
application/json:
schema:
type: Array
items:
type: string
"/api/v1/hotels/filter":
post:
summary: Hotel search filter
tags:
- Hotels
parameters: []
responses:
'200':
description: only one Hotels found
Copy link

Choose a reason for hiding this comment

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

medium

The description "only one Hotels found" is too specific and likely incorrect for a general filter endpoint, which can return zero, one, or multiple hotels. A more accurate description would be "A list of hotels matching the filter criteria."

          description: A list of hotels matching the filter criteria

content:
application/json:
schema:
type: Array
items:
type: object
properties:
name:
type: string
title:
type: string
description:
type: string
country:
type: string
city:
type: string
nullable: true
state:
type: string
nullable: true
required:
- name
- title
- description
requestBody:
content:
application/json:
schema:
type: object
properties:
name:
type: string
title:
type: string
description:
type: string
country:
type: string
city:
type: string
state:
type: string
description: hotel filter
"/api/v1/routes/{id}":
get:
summary: Retrieves a route by ID
Expand Down Expand Up @@ -619,10 +692,7 @@ paths:
'404':
description: route not found
servers:
- url: http://{defaultHost}
variables:
defaultHost:
default: localhost:3000
- url: "/"
components:
schemas:
Airline:
Expand Down