Skip to content

Commit

Permalink
Merge pull request #3489 from magda-io/issue-3408
Browse files Browse the repository at this point in the history 
#3408: auto reset search dataset result cache when login status changes & others
  • Loading branch information
@t83714
t83714 authored Oct 18, 2023
2 parents acc6abf + b7e84c4 commit af6f01e
Show file tree
Hide file tree
Showing 19 changed files with 334 additions and 134 deletions.
3 changes: 3 additions & 0 deletions CHANGES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@

- #3485: New feature for registry records manager: able to search record by keywords & added records list view
- Display organizational unit ID info in the user general info panel
- Improved Registry event processing performance
- #3100: Conenctor SDK: Allow runtime filter code to be supply to connectors via Helm config
- Fixed an issue with docker build utiliy
- #3408: auto reset search dataset result cache when login status changes
- Allow users to open datasets & distributions in "Raw Record Editor" when the user has edit permission

## v2.3.1

Expand Down
6 changes: 6 additions & 0 deletions magda-migrator-registry-db/sql/V2_8__optimise_event_aspect_query.sql
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
-- this will make planner to use gin index for distributions rather than seq scan
-- event web hook processing will be much faster
CREATE INDEX recordsaspects_data_distributions_gin_with_aspect_cond_idx
ON recordaspects USING gin
((data -> 'distributions'::text))
WHERE aspectid = 'dataset-distributions';
4 changes: 2 additions & 2 deletions magda-registry-api/src/main/resources/application.conf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ akka {
# `Timeout-Access` header to the request, which enables programmatic
# customization of the timeout period and timeout response for each
# request individually.
request-timeout = 30s
request-timeout = 60s

# The time after which an idle connection will be automatically closed.
# Set to `infinite` to completely disable idle connection timeouts.
Expand Down Expand Up @@ -78,7 +78,7 @@ db-query {

# the default timeout setting used for most common queries that serve rest APIs.
# we put it in place via DBSession.queryTimeout(seconds: Int)
default-timeout = "90s"
default-timeout = "180s"

# Similar to `default-timeout`. But this setting allow us to use different (often longer) timeout settings for long queries.
# e.g. Trim operations.
Expand Down
20 changes: 19 additions & 1 deletion magda-registry-api/src/main/scala/au/csiro/data61/magda/registry/RecordPersistence.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1772,8 +1772,25 @@ class DefaultRecordPersistence(config: Config)
sqls"order by sequence DESC"
else sqls"order by sequence"

/**
* Why we need to "fence" the query?
* select * from (
* select xxx,xx from xxx where xxx
* order by xx
* ) as tmp_tab order by xx offset xxx limit xxx
*
* When both order by & (limit or offset) involved might make planner
* give up using full text index and choose to use sequence idx for filtering & sorting
*
* The "fence" make it always use full text index and sort by key sequence
* sort by key sequence could be very expense but in real world they are much less expensive than full text search.
* Our records table's frequent update nature might also contribute to it (planer won't get accurate cost estimate)
* vacuum analyze won't help either
*
*/
val result =
sql"""select Records.sequence as sequence,
sql"""select * from (
select Records.sequence as sequence,
Records.recordId as recordId,
Records.name as recordName,
(select array_agg(aspectId) from RecordAspects ${SQLSyntax
Expand All @@ -1787,6 +1804,7 @@ class DefaultRecordPersistence(config: Config)
Records.tenantId as tenantId
from Records
${SQLSyntax.where(SQLUtils.toAndConditionOpt(whereClauseParts: _*))}
${orderBy}) as tmp_results
${orderBy}
offset ${start.getOrElse(0)}
limit ${limit + 1}"""
Expand Down
100 changes: 93 additions & 7 deletions magda-typescript-common/src/test/JsonConnector.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,79 @@ describe("JsonConnector", () => {
});
});

it("Will cralw only one dataset (`blah2-dataset-2`) and one distribution with `customJsFilterCode`", () => {
const { scope, connector } = setupCrawlTest({
customJsFilterCode: `if(type === 'Dataset' && jsonData.blah != 'blah2-dataset-2') {
return false;
} else {
return true;
}`
});
const receivedRecords: Record[] = [];

scope
.persist()
.put(new RegExp("/records"))
.reply(200, (uri: string, requestBody: any) => {
receivedRecords.push(requestBody);
});
scope.delete(/.*/).reply(202);

return connector.run().then((result) => {
scope.done();
expect(receivedRecords.length).to.equal(3);
const datasets = receivedRecords.filter(
(record) => !!record.aspects["dataset-distributions"]
);
expect(datasets.length).to.equal(1);
expect(datasets[0].name).to.equal("blah2-dataset-2");
const dists = receivedRecords.filter(
(record) => !record.aspects["dataset-distributions"]
);
expect(dists.length).to.equal(2);
expect(dists.map((dist) => dist.name)).to.include.members([
"blah-dist-1",
"blah-dist-2"
]);
});
});

it("Will cralw only one dataset (`blah2-dataset-1`) with `customJsFilterCode`", () => {
const { scope, connector } = setupCrawlTest({
customJsFilterCode: `if(type === 'Dataset' && jsonData.blah != 'blah-dataset-1') {
return false;
} else if(type === 'Distribution' && jsonData.blah != 'blah-dist-2') {
return false;
}else {
return true;
}`
});
const receivedRecords: Record[] = [];

scope
.persist()
.put(new RegExp("/records"))
.reply(200, (uri: string, requestBody: any) => {
receivedRecords.push(requestBody);
});
scope.delete(/.*/).reply(202);

return connector.run().then((result) => {
scope.done();
expect(receivedRecords.length).to.equal(2);
const datasets = receivedRecords.filter(
(record) => !!record.aspects["dataset-distributions"]
);
expect(datasets.length).to.equal(1);
expect(datasets[0].name).to.equal("blah-dataset-1");
const dists = receivedRecords.filter(
(record) => !record.aspects["dataset-distributions"]
);
expect(dists.length).to.equal(1);
expect(dists[0].name).to.equal("blah-dist-2");
});
});

function setupCrawlTest(
config: FakeConnectorSourceConfig = {},
transformerConfig: JsonTransformerOptions = {} as JsonTransformerOptions
Expand Down Expand Up @@ -362,22 +435,23 @@ class FakeJsonTransformer extends JsonTransformer {
}

getNameFromJsonOrganization(jsonOrganization: any): string {
return "name";
return jsonOrganization?.name ? jsonOrganization.name : "name";
}
getNameFromJsonDataset(jsonDataset: any): string {
return "name";
return jsonDataset?.name ? jsonDataset.name : "name";
}
getNameFromJsonDistribution(
jsonDistribution: any,
jsonDataset: any
): string {
return "name";
return jsonDistribution?.name ? jsonDistribution.name : "name";
}
}

type FakeConnectorSourceConfig = {
extras?: JsonConnectorConfigExtraMetaData;
presetRecordAspects?: JsonConnectorConfigPresetAspect[];
customJsFilterCode?: string;
};

class FakeConnectorSource implements ConnectorSource {
Expand All @@ -386,6 +460,7 @@ class FakeConnectorSource implements ConnectorSource {
readonly hasFirstClassOrganizations: boolean = false;
presetRecordAspects: JsonConnectorConfigPresetAspect[] = null;
extras: JsonConnectorConfigExtraMetaData = null;
customJsFilterCode: string = "";

constructor(config: FakeConnectorSourceConfig = {}) {
if (config.extras) {
Expand All @@ -394,6 +469,9 @@ class FakeConnectorSource implements ConnectorSource {
if (config.presetRecordAspects) {
this.presetRecordAspects = config.presetRecordAspects;
}
if (config.customJsFilterCode) {
this.customJsFilterCode = config.customJsFilterCode;
}
}

getJsonDataset(id: string): Promise<any> {
Expand Down Expand Up @@ -422,24 +500,32 @@ class FakeConnectorSource implements ConnectorSource {
getJsonDatasets(): AsyncPage<any[]> {
return AsyncPage.single([
{
blah: "blah"
blah: "blah-dataset-1",
name: "blah-dataset-1"
},
{
blah: "blah"
blah: "blah2-dataset-2",
name: "blah2-dataset-2"
}
]);
}
getJsonFirstClassOrganizations(): AsyncPage<any[]> {
return AsyncPage.single([
{
blah: "blah"
blah: "blah-org-1",
name: "blah-org-1"
}
]);
}
getJsonDistributions(): AsyncPage<any[]> {
return AsyncPage.single([
{
blah: "blah"
blah: "blah-dist-1",
name: "blah-dist-1"
},
{
blah: "blah-dist-2",
name: "blah-dist-2"
}
]);
}
Expand Down
138 changes: 69 additions & 69 deletions magda-web-client/src/AppContainer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,10 @@ import Notification from "./Components/Common/Notification";
import { hideTopNotification } from "./actions/topNotificationAction";

import Routes from "./Routes";

import { Medium } from "./Components/Common/Responsive";

import { Route, Switch } from "react-router-dom";

import { getPluginFooter } from "./externalPluginComponents";

import inPopUpMode from "./helpers/inPopUpMode";
import "./AppContainer.scss";

const ExternalFooterComponent = getPluginFooter();
Expand All @@ -31,6 +28,7 @@ class AppContainer extends React.Component {
}

render() {
const isPopUp = inPopUpMode();
return (
<MagdaDocumentTitle>
<div className="au-grid wrapper">
Expand All @@ -54,71 +52,73 @@ class AppContainer extends React.Component {
<Routes />
</div>

<Switch>
{/** turn off top margin for home page only */}
<Route
exact
path="/home"
render={() =>
ExternalFooterComponent ? (
<ExternalFooterComponent
noTopMargin={true}
footerMediumNavs={
this.props.footerMediumNavs
}
footerSmallNavs={
this.props.footerSmallNavs
}
footerCopyRightItems={
this.props.footerCopyRightItems
}
/>
) : (
<Footer noTopMargin={true} />
)
}
/>
<Route
path="/settings(/)*(.)*"
render={() =>
ExternalFooterComponent ? (
<ExternalFooterComponent
footerMediumNavs={
this.props.footerMediumNavs
}
footerSmallNavs={
this.props.footerSmallNavs
}
footerCopyRightItems={
this.props.footerCopyRightItems
}
/>
) : (
<Footer noTopMargin={true} />
)
}
/>
<Route
path="/*"
render={() =>
ExternalFooterComponent ? (
<ExternalFooterComponent
footerMediumNavs={
this.props.footerMediumNavs
}
footerSmallNavs={
this.props.footerSmallNavs
}
footerCopyRightItems={
this.props.footerCopyRightItems
}
/>
) : (
<Footer />
)
}
/>
</Switch>
{!isPopUp && (
<Switch>
{/** turn off top margin for home page only */}
<Route
exact
path="/home"
render={() =>
ExternalFooterComponent ? (
<ExternalFooterComponent
noTopMargin={true}
footerMediumNavs={
this.props.footerMediumNavs
}
footerSmallNavs={
this.props.footerSmallNavs
}
footerCopyRightItems={
this.props.footerCopyRightItems
}
/>
) : (
<Footer noTopMargin={true} />
)
}
/>
<Route
path="/settings(/)*(.)*"
render={() =>
ExternalFooterComponent ? (
<ExternalFooterComponent
footerMediumNavs={
this.props.footerMediumNavs
}
footerSmallNavs={
this.props.footerSmallNavs
}
footerCopyRightItems={
this.props.footerCopyRightItems
}
/>
) : (
<Footer noTopMargin={true} />
)
}
/>
<Route
path="/*"
render={() =>
ExternalFooterComponent ? (
<ExternalFooterComponent
footerMediumNavs={
this.props.footerMediumNavs
}
footerSmallNavs={
this.props.footerSmallNavs
}
footerCopyRightItems={
this.props.footerCopyRightItems
}
/>
) : (
<Footer />
)
}
/>
</Switch>
)}

{this.props.topNotification.visible ? (
<Notification
Expand Down
Loading

0 comments on commit af6f01e

Please sign in to comment.