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

PM- added testing case for sorting data from storage #1458

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,25 @@ public interface IStorageUtilityIndexed<E extends Externalizable> {
*/
Vector<E> getRecordsForValues(String[] metaFieldNames, Object[] values);

/**
* Load multiple record objects from storage at one time from a list of record ids in sorted way.
* <p>
* If the provided recordMap already contains entries for any ids, it is _not_
* required for them to be retrieved from storage again.
*
* metaFieldNames Array of metadata field names to match
* values Array of values corresponding to the field names
* orderby String in format "fieldName [ASC|DESC]". If null or empty, records are returned unsorted.
* ASC/DESC is case-insensitive. If direction is omitted, defaults to ASC.
* @return Vector of records matching the criteria, sorted if orderby is valid
*
* @throws IllegalArgumentException If the argument is there or spelling mistake in ASC|DESC or there
* is parameter missing for the orderby
*/

Vector<E> getSortedRecordsForValues(String[] metaFieldNames, Object[] values,String orderby) throws IllegalArgumentException;


/**
* Load multiple record objects from storage at one time from a list of record ids.
* <p>
Expand All @@ -250,7 +269,6 @@ public interface IStorageUtilityIndexed<E extends Externalizable> {
* or another exception, but they should anticipate that
* they may need to clean up if the bulk read doesn't complete
*/

void bulkRead(LinkedHashSet<Integer> cuedCases, HashMap<Integer, E> recordMap)
throws RequestAbandonedException;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,11 @@
import java.io.DataInputStream;
import java.io.DataOutputStream;
import java.io.IOException;
import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.Hashtable;
Expand Down Expand Up @@ -173,6 +176,69 @@ public Vector<T> getRecordsForValues(String[] metaFieldNames, Object[] values) {
return matches;
}

@Override
public Vector<T> getSortedRecordsForValues(String[] metaFieldNames, Object[] values, String orderby) throws IllegalArgumentException{
Vector<T> matches = new Vector<>();
List<Integer> idMatches = getIDsForValues(metaFieldNames, values);

for (Integer id : idMatches) {
matches.add(read(id));
}

if (orderby == null || orderby.trim().isEmpty()) {
return matches; // No sorting required
}

// Parse orderBy into field and direction
if (!orderby.matches("^\\w+\\s*(ASC|DESC)?\\s*$")) {
throw new IllegalArgumentException("Invalid format");
}
String[] orderParts = orderby.trim().split("\\s+");
String fieldName = orderParts[0];
final boolean isAscending = orderParts.length <= 1 || !orderParts[1].equalsIgnoreCase("DESC");
// Perform sorting using reflection for field access
Collections.sort(matches, new Comparator<T>() {
@Override
public int compare(T record1, T record2) {
try {
Object value1 = getFieldValue(record1, fieldName);
Object value2 = getFieldValue(record2, fieldName);
if (value1 == null && value2 == null) {
return 0;
}
if (value1 == null) {
return isAscending ? -1 : 1;
}
if (value2 == null) {
return isAscending ? 1 : -1;
}
if (value1 instanceof Comparable && value2 instanceof Comparable) {
int comparison = ((Comparable) value1).compareTo(value2);
return isAscending ? comparison : -comparison;
}
coderabbitai[bot] marked this conversation as resolved.
Show resolved Hide resolved
} catch (Exception ignore) {
}
return 0; // Default to no ordering if field access fails
}
});

return matches;
}
private Object getFieldValue(T record, String fieldName) throws Exception {
Class<?> clazz = record.getClass();
while (clazz != null) {
try {
Field field = clazz.getDeclaredField(fieldName);
field.setAccessible(true);
return field.get(record);
} catch (NoSuchFieldException e) {
clazz = clazz.getSuperclass();
}
}
throw new NoSuchFieldException("Field '" + fieldName + "' not found in class hierarchy");
}


@Override
public int add(T e) {
data.put(DataUtil.integer(curCount), e);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
package org.javarosa.core.storage;

import org.javarosa.core.services.storage.IStorageUtilityIndexed;
import org.javarosa.core.services.storage.util.DummyIndexedStorageUtility;
import org.javarosa.core.util.Interner;
import org.javarosa.core.util.externalizable.LivePrototypeFactory;
import org.javarosa.core.util.externalizable.PrototypeFactory;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -32,6 +28,7 @@ public abstract class IndexedStorageUtilityTests {
Shoe[] eightSizesOfWomensNikes;

Shoe[] fiveSizesOfMensVans;
Shoe[] fiveSortedSizesOfMensAdidas;

protected abstract IStorageUtilityIndexed<Shoe> createStorageUtility();

Expand All @@ -58,6 +55,11 @@ public void setupStorageContainer() {
fiveSizesOfMensVans[i] =
new Shoe("vans", "mens", String.valueOf(i + 1));
}

fiveSortedSizesOfMensAdidas = new Shoe[5];
for (int i = 0; i < 5; ++i) {
fiveSortedSizesOfMensAdidas[i]= new Shoe("adidas", "mens", String.valueOf(5 - i));
}
}

@Test
Expand Down Expand Up @@ -126,6 +128,25 @@ public void testBulkMetaMatching() {

Vector<Shoe> matchedRecords = storage.getRecordsForValues(new String[]{Shoe.META_BRAND, Shoe.META_STYLE}, new String[]{"nike", "mens"});
Assert.assertEquals("Failed index match [brand,style][nike,mens]", getIdsFromModels(tenSizesOfMensNikes), getIdsFromModels(matchedRecords.toArray(new Shoe[]{})));

Vector<Shoe> matchedSortedRecords = storage.getSortedRecordsForValues(new String[]{Shoe.META_BRAND, Shoe.META_STYLE}, new String[]{"adidas", "mens"},Shoe.META_SIZE+" DESC");
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the expected outcome for function calls without select params - storage.getSortedRecordsForValues(new String[]{}, new String[]{},Shoe.META_SIZE+" DESC"); ?

Copy link
Author

Choose a reason for hiding this comment

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

revert the array as it is

Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a test case for it.

Copy link
Author

Choose a reason for hiding this comment

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

added new commit in 9007023

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see the test case for empty selections array as mentioned above in your commit.

Assert.assertArrayEquals("Failed index match [brand,style][adidas,mens]", fiveSortedSizesOfMensAdidas,matchedSortedRecords.toArray());

Vector<Shoe> matchedAscSortedRecords = storage.getSortedRecordsForValues(new String[]{Shoe.META_BRAND, Shoe.META_STYLE}, new String[]{"vans", "mens"},Shoe.META_SIZE+" ASC");
Assert.assertArrayEquals("Failed index match [brand,style][adidas,mens]", matchedAscSortedRecords.toArray(),fiveSizesOfMensVans);

try {
Vector<Shoe> matchedDescSortedRecordsWithoutKey = storage.getSortedRecordsForValues(new String[]{Shoe.META_BRAND, Shoe.META_STYLE}, new String[]{"adidas", "mens"}, " DESC");
Assert.assertArrayEquals("Failed the sorted match [brand,style][adidas,mens]", matchedDescSortedRecordsWithoutKey.toArray(), fiveSortedSizesOfMensAdidas);
}catch (IllegalArgumentException e){
System.out.println(e.getMessage());
}
try {
Vector<Shoe> matchedAscSortedRecordsTest = storage.getSortedRecordsForValues(new String[]{Shoe.META_BRAND, Shoe.META_STYLE}, new String[]{"vans", "mens"}, Shoe.META_SIZE + " ASSC");
Assert.assertArrayEquals("Failed the sorted match [brand,style][adidas,mens]", matchedAscSortedRecordsTest.toArray(), fiveSizesOfMensVans);
}catch (IllegalArgumentException e){
System.out.println(e.getMessage());
}
}

@Test
Expand All @@ -142,6 +163,7 @@ private void writeBulkSets() {
writeAll(tenSizesOfMensNikes);
writeAll(eightSizesOfWomensNikes);
writeAll(fiveSizesOfMensVans);
writeAll(fiveSortedSizesOfMensAdidas);
shubham1g5 marked this conversation as resolved.
Show resolved Hide resolved
}

Set<Integer> getIdsFromModels(Shoe[] shoes) {
Expand Down
Loading