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

Changed search system to keyset-based #212

Merged
merged 24 commits into from
May 24, 2022
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
9d74a39
Merge pull request #230 from NIAEFEUP/develop
DoStini Apr 9, 2022
d3e4dec
Merge pull request #232 from NIAEFEUP/develop
DoStini Apr 11, 2022
8de31ad
Fix edit not working when offer is hidden
DoStini Apr 13, 2022
d57275f
Merge pull request #233 from NIAEFEUP/hotfix/edit-offer
DoStini Apr 13, 2022
c4c40cf
Changed get offers validators to include lastOfferId instead of offset
BrunoRosendo Feb 28, 2022
84a13df
Changed offer search to keyset instead of offset-limit
BrunoRosendo Feb 28, 2022
9a200f7
Validator for last offer search criteria
BrunoRosendo Mar 2, 2022
3401fc1
Changed conditions to functions, to fix wrong dates
BrunoRosendo Mar 3, 2022
75858d7
Removed offset validator in GET /offers
BrunoRosendo Mar 3, 2022
effcd7a
Fixed bug when passing offer IDs
BrunoRosendo Mar 4, 2022
9d36dcf
Tests for new search
BrunoRosendo Mar 5, 2022
81ea8c9
Changed lastOfferId parameter to queryToken and offer filters
BrunoRosendo Mar 14, 2022
7ae7aab
Implemented queryToken strategy
BrunoRosendo Mar 17, 2022
442e108
Fixed tests to account for queryTokens
BrunoRosendo Mar 18, 2022
634b39e
Tests for new queryToken
BrunoRosendo Mar 18, 2022
7477941
Changed queryToken from once per offer to once per search
BrunoRosendo Mar 20, 2022
8818401
Fixed test by adding expired_test_offer
BrunoRosendo May 20, 2022
3f0ad71
Extracted the construction of search query and aggregation to seperat…
BrunoRosendo May 21, 2022
4d2f4da
Changed the names and documentation of search query builders
BrunoRosendo May 21, 2022
00f818d
encodeQueryToken() now receives fields instead of internal offer object
BrunoRosendo May 22, 2022
32871b8
Using queryToken to pass search query parameters
BrunoRosendo May 22, 2022
e05e881
Changed validGetQueryToken to use the query token's value
BrunoRosendo May 22, 2022
a2751ae
Fixed and added queryToken tests according to the new implementation
BrunoRosendo May 22, 2022
f445589
Extracted repeated code to selectSearchOffers() method
BrunoRosendo May 24, 2022
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
5 changes: 5 additions & 0 deletions package-lock.json

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

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"dependencies": {
"@babel/plugin-proposal-optional-chaining": "^7.12.7",
"babel": "^6.23.0",
"base64url": "^3.0.1",
imnotteixeira marked this conversation as resolved.
Show resolved Hide resolved
"bcrypt": "^5.0.1",
"cloudinary": "^1.24.0",
"dotenv-flow": "^3.0.0",
Expand Down
2 changes: 1 addition & 1 deletion src/api/middleware/company.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export const verifyMaxConcurrentOffersOnEdit = async (req, res, next) => {

try {

const offer = await (new OfferService()).getOfferById(req.params.offerId, req.user);
const offer = await (new OfferService()).getOfferById(req.params.offerId, req.targetOwner, req.hasAdminPrivileges);

if (!offer)
throw new APIError(HTTPStatus.NOT_FOUND, ErrorTypes.VALIDATION_ERROR, ValidationReasons.OFFER_NOT_FOUND(req.params.offerId));
Expand Down
49 changes: 35 additions & 14 deletions src/api/middleware/validators/offer.js
Original file line number Diff line number Diff line change
Expand Up @@ -312,21 +312,23 @@ const publishEndDateEditableLimit = async (publishEndDateCandidate, { req }) =>
return true;
};

const existingOfferId = async (offerId, { req }) => {
try {
const offer = await (new OfferService()).getOfferById(offerId, req.targetOwner, req.hasAdminPrivileges);
if (!offer) throw new Error(ValidationReasons.OFFER_NOT_FOUND(offerId));
} catch (err) {
console.error(err);
throw err;
}

return true;
};

export const isExistingOffer = useExpressValidators([
param("offerId", ValidationReasons.DEFAULT)
.exists().withMessage(ValidationReasons.REQUIRED).bail()
.custom(isObjectId).withMessage(ValidationReasons.OBJECT_ID).bail()
.custom(async (offerId, { req }) => {
try {
const offer = await (new OfferService()).getOfferById(offerId, req.targetOwner, req.hasAdminPrivileges);
if (!offer) throw new Error(ValidationReasons.OFFER_NOT_FOUND(offerId));
} catch (err) {
console.error(err);
throw err;
}

return true;
}),
.custom(existingOfferId),
]);

export const edit = useExpressValidators([
Expand Down Expand Up @@ -440,11 +442,30 @@ export const setDefaultValuesCreate = (req, res, next) => {
return next();
};

const validGetQueryToken = async (queryToken, { req }) => {
try {
const { id, score } = (new OfferService()).decodeQueryToken(queryToken);
if (!isObjectId(id)) throw new Error(ValidationReasons.OBJECT_ID);
await existingOfferId(id, { req });

if (req.query.value) {
if (isNaN(score)) throw new Error(ValidationReasons.NUMBER);
if (score < 0) throw new Error(ValidationReasons.MIN(0));
}

} catch (err) {
console.error(err);
throw new Error(ValidationReasons.INVALID_QUERY_TOKEN);
}

return true;
};

export const get = useExpressValidators([
query("offset", ValidationReasons.DEFAULT)
query("queryToken", ValidationReasons.DEFAULT)
.optional()
.isInt({ min: 0 }).withMessage(ValidationReasons.INT)
.toInt(),
.isString().withMessage(ValidationReasons.STRING).bail()
.custom(validGetQueryToken),

query("limit")
.optional()
Expand Down
2 changes: 2 additions & 0 deletions src/api/middleware/validators/validationReasons.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const ValidationReasons = Object.freeze({
DATE: "must-be-ISO8601-date",
INT: "must-be-int",
BOOLEAN: "must-be-boolean",
NUMBER: "must-be-number",
IN_ARRAY: (vals, field) => `${field ? `${field}:` : ""}must-be-in:[${vals}]`,
ARRAY_SIZE: (min, max) => `size-must-be-between:[${min},${max}]`,
OBJECT_ID: "must-be-a-valid-id",
Expand All @@ -37,6 +38,7 @@ const ValidationReasons = Object.freeze({
OFFER_EXPIRED: (id) => `offer-expired:${id}`,
NOT_OFFER_OWNER: (id) => `not-offer-owner:${id}`,
OFFER_EDIT_PERIOD_OVER: (value) => `offer-edit-period-over:${value}-hours`,
INVALID_QUERY_TOKEN: "invalid-query-token",
JOB_MIN_DURATION_NOT_SPECIFIED: "job-max-duration-requires-job-min-duration",
REGISTRATION_FINISHED: "registration-already-finished",
REGISTRATION_NOT_FINISHED: "registration-not-finished-yet",
Expand Down
9 changes: 5 additions & 4 deletions src/api/routes/offer.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,21 @@ export default (app) => {
router.use(offerMiddleware.setTargetOwner);

/**
* Gets all currently active offers (without filtering, for now)
* supports offset and limit as query params
* Gets active offers based on passed filters and full-text search.
* Returns the offers found and a queryToken used for pagination.
* Also takes queryToken and limit as query params.
*/
router.get("/", validators.get, async (req, res, next) => {
try {
const offers = await (new OfferService()).get(
const resultsAndQueryToken = await (new OfferService()).get(
{
...req.query,
showHidden: req?.query?.showHidden && req.hasAdminPrivileges,
showAdminReason: req.hasAdminPrivileges
}
);

return res.json(offers);
return res.json(resultsAndQueryToken);
} catch (err) {
console.error(err);
return next(err);
Expand Down
20 changes: 11 additions & 9 deletions src/models/Offer.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,22 +146,24 @@ function validateDescription(value) {
/**
* Currently active Offers (publish date was before Date.now and end date is after Date.now)
*/
OfferSchema.statics.filterCurrent = () => ({
publishDate: {
$lte: new Date(Date.now()),
},
publishEndDate: {
$gt: new Date(Date.now()),
},
});
OfferSchema.query.current = function() {
return this.where({
publishDate: {
$lte: new Date(Date.now()),
},
publishEndDate: {
$gt: new Date(Date.now()),
},
});
return this.where(this.model.filterCurrent());
};

/**
* Currently active and non-hidden Offers
*/
OfferSchema.statics.filterNonHidden = () => ({ isHidden: false });
OfferSchema.query.withoutHidden = function() {
return this.where({ isHidden: false });
return this.where(this.model.filterNonHidden());
};

const Offer = mongoose.model("Offer", OfferSchema);
Expand Down
110 changes: 100 additions & 10 deletions src/services/offer.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import mongoose from "mongoose";
import Company from "../models/Company.js";
import Offer from "../models/Offer.js";
import Account from "../models/Account.js";
import EmailService from "../lib/emailService.js";
import { OFFER_DISABLED_NOTIFICATION } from "../email-templates/companyOfferDisabled.js";
import OfferConstants from "../models/constants/Offer.js";
import base64url from "base64url";

const { ObjectId } = mongoose.Types;

class OfferService {
// TODO: Use typedi or similar
Expand Down Expand Up @@ -192,29 +196,91 @@ class OfferService {

/**
* Fetches offers according to specified options
* Learn more about keyset search here: https://github.com/NIAEFEUP/nijobs-be/issues/129
*
* @param {*} options
* value: Text to use in full-text-search
* offset: Point to start looking (and limiting)
* queryToken: Id of the last fetched offer
BrunoRosendo marked this conversation as resolved.
Show resolved Hide resolved
* limit: How many offers to show
* jobType: Array of jobTypes allowed
*/
get({ value = "", offset = 0, limit = OfferService.MAX_OFFERS_PER_QUERY, showHidden = false, showAdminReason = false, ...filters }) {
async get({ value = "", queryToken = null, limit = OfferService.MAX_OFFERS_PER_QUERY,
BrunoRosendo marked this conversation as resolved.
Show resolved Hide resolved
showHidden = false, showAdminReason = false, ...filters }) {

const offers = (value ? Offer.find(
{ "$and": [this._buildFilterQuery(filters), { "$text": { "$search": value } }] }, { score: { "$meta": "textScore" } }
) : Offer.find(this._buildFilterQuery(filters))).current();
const {
id: lastOfferId,
score: lastOfferScore
} = queryToken ? this.decodeQueryToken(queryToken) : {};

if (!showHidden) offers.withoutHidden();
const offers = lastOfferId && value ?
BrunoRosendo marked this conversation as resolved.
Show resolved Hide resolved
this._buildSearchAggregation(lastOfferId, lastOfferScore, value, showAdminReason, showHidden, filters)
:
this._buildSearchQuery(lastOfferId, value, showAdminReason, showHidden, filters);

const offersQuery = offers
.sort(value ? { score: { "$meta": "textScore" } } : undefined)
.skip(offset)
const results = await offers
.sort(value ? { score: { "$meta": "textScore" }, _id: 1 } : { _id: 1 })
BrunoRosendo marked this conversation as resolved.
Show resolved Hide resolved
.limit(limit)
;

return showAdminReason ? offersQuery : offersQuery.select("-adminReason");
return results.length > 0 ?
{ results, queryToken: this.encodeQueryToken(results[results.length - 1]) }
:
{ results };
}

/**
* Builds a search query with a regular find().
BrunoRosendo marked this conversation as resolved.
Show resolved Hide resolved
* Cannot be used when loading more offers and searching with full-text search at the same time.
* Otherwise, use _buildSearchAggregation().
*/
_buildSearchQuery(lastOfferId, value, showAdminReason, showHidden, filters) {
let offers;
if (lastOfferId) {

offers = Offer.find({ "$and": [
this._buildFilterQuery(filters),
{ _id: { "$gt": ObjectId(lastOfferId) } }
] });

} else {

offers = (value ? Offer.find({ "$and": [
this._buildFilterQuery(filters),
{ "$text": { "$search": value } }
] }, { score: { "$meta": "textScore" } }

) : Offer.find(this._buildFilterQuery(filters)));
}

offers.current();
if (!showHidden) offers.withoutHidden();
if (!showAdminReason) offers.select("-adminReason");
BrunoRosendo marked this conversation as resolved.
Show resolved Hide resolved

return offers;
}

/**
* Builds a search aggregation with aggregate().
* Only use this when loading more offers and searching with full-text search at the same time.
* Otherwise, use _buildSearchQuery().
*/
_buildSearchAggregation(lastOfferId, lastOfferScore, value, showAdminReason, showHidden, filters) {
BrunoRosendo marked this conversation as resolved.
Show resolved Hide resolved
return Offer.aggregate([
{ $match: { $text: { $search: value } } },
{ $match: filters },
{ $addFields: {
score: { $meta: "textScore" },
adminReason: { $cond: [showAdminReason, "$adminReason", "$$REMOVE"] }
} },
{ $match: { "$or": [
{ score: { "$lt": lastOfferScore } },
{ score: lastOfferScore, _id: { "$gt": ObjectId(lastOfferId) } }
] } },
{ $match: Offer.filterCurrent() },
{ $match: showHidden ? {} : Offer.filterNonHidden() }
]);
}

_buildFilterQuery(filters) {
if (!filters || !Object.keys(filters).length) return {};

Expand Down Expand Up @@ -262,6 +328,30 @@ class OfferService {
return constraints.length ? { "$and": constraints } : {};
}

/**
* Encodes a query token, by taking the offer's id and FTS score (if present) and decoding in safe url base64
* @param {*} offer
*/
encodeQueryToken(offer) {
return base64url.encode(JSON.stringify({
id: offer._id,
score: offer.score || offer._doc?.score
BrunoRosendo marked this conversation as resolved.
Show resolved Hide resolved
}));
}

/**
* Decodes a query token, extracting the lastOffer's ID and FTS score
* @param {*} queryToken
*/
decodeQueryToken(queryToken) {
const tokenInfo = JSON.parse(base64url.decode(queryToken));

return {
...tokenInfo,
score: Number(tokenInfo.score)
};
}

/**
* Checks whether a given offer is visible to a specific userCompanyId.
* Unpublished/Unactive offers may still be visible
Expand Down
Loading