-
-
Notifications
You must be signed in to change notification settings - Fork 499
Add support to get metatada resource json object into the index. #9036
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
base: main
Are you sure you want to change the base?
Add support to get metatada resource json object into the index. #9036
Conversation
81b1b65
to
d584984
Compare
...ters/src/main/java/org/fao/geonet/kernel/harvest/harvester/geonet/BaseGeoNetworkAligner.java
Outdated
Show resolved
Hide resolved
d584984
to
bf60288
Compare
Changed some of the conversion tool for xml to json and vice versa so that they support json arrays and other json nodes. Also fixed issue with indexer not being triggered after resources are uploaded.
bf60288
to
52750a8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested, works as expected
…into support_for_metadata_filestore_properties_in_index
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added some comments and suggestions. Can you check them if make sense, please?
.checkForSimilar() | ||
.build(); | ||
assertFalse(diff.toString(), diff.hasDifferences()); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm adding here a new test for JSON object.
} | |
} | |
/** | |
* Tests the conversion of a JSON object to an XML representation and validates | |
* the resulting XML against an expected XML representation. | |
*/ | |
@Test | |
public void testObjectConversion() throws Exception { | |
String jsonString = "{\"Test\":\"value\", \"nestedObject\": {\"nestedField\": 433}}"; | |
String expectedElement = "<root><Test>value</Test><nestedObject><nestedField>433</nestedField></nestedObject></root>"; | |
Element xmlFromJSON = Xml.getXmlFromJSON(jsonString); | |
Diff diff = DiffBuilder | |
.compare(Input.fromString(expectedElement)) | |
.withTest(Input.fromString(Xml.getString(xmlFromJSON))) | |
.withNodeMatcher(new DefaultNodeMatcher(ElementSelectors.byName)) | |
.normalizeWhitespace() | |
.ignoreComments() | |
.checkForSimilar() | |
.build(); | |
assertFalse(diff.toString(), diff.hasDifferences()); | |
} |
*/ | ||
public static String getString(Node data) { | ||
try { | ||
TransformerFactory tf = TransformerFactory.newInstance(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it isn't a good idea from the point of view of the performance to create a new TransformerFactory
every time the method is called. Shouldn't we use TransformerFactoryFactory.getTransformerFactory()
instead that uses a singleton?
TransformerFactory tf = TransformerFactory.newInstance(); | |
TransformerFactory tf = TransformerFactoryFactory.getTransformerFactory(); |
ObjectMapper objectMapper = new ObjectMapper(); | ||
objectMapper.setSerializationInclusion(JsonInclude.Include.NON_NULL); | ||
objectMapper.disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS); | ||
|
||
// Exclude fields with the index ignore annotation | ||
objectMapper.setAnnotationIntrospector(new JacksonAnnotationIntrospector() { | ||
@Override | ||
public boolean hasIgnoreMarker(AnnotatedMember member) { | ||
return member.hasAnnotation(IndexIgnore.class) || super.hasIgnoreMarker(member); | ||
} | ||
|
||
@Override | ||
public PropertyName findNameForSerialization(Annotated annotated) { | ||
if (annotated.hasAnnotation(IndexIgnore.class)) return null; | ||
return super.findNameForSerialization(annotated); | ||
} | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creating an ObjectMapper
every time this method is called is inefficient. Maybe we can move it to be a member of the class?
// Index the record so that the resources are included | ||
v.indexMetadata(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why in MEFVisitor
v.indexMetadata()
is called in visit()
but in MEF2Visitor
is called in handleXml()
method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It just needed to follow the handleBin()
call which was already in visit()
for MEFVisitor
and handleXml()
for MEF2Visitor
before these changes
Add support to get metatada resource json object into the index.
Changed some of the conversion tool for xml to json and vice versa so that they support json arrays and other json nodes. Also fixed issue with indexer not being triggered after resources are uploaded.
Indexing resource information such as size and last modified data is helpful to be able to search on resources that were recently modified or to be able to search or summarize metadata content size quickly.
This is sample json that would be added to the index.
Checklist
main
branch, backports managed with labelREADME.md
filespom.xml
dependency management. Update build documentation with intended library use and library tutorials or documentation