Skip to content

Commit 95e34ee

Browse files
Merge pull request #401 from QRun-IO/feature/tiebreak-meta-data-producer-sort-by-name
fix(metaDataProduction): add class simple name as a tie-breaker when …
2 parents 24cb18c + 00d1fd4 commit 95e34ee

File tree

2 files changed

+246
-7
lines changed

2 files changed

+246
-7
lines changed

qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/MetaDataProducerHelper.java

Lines changed: 53 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,18 @@
5757

5858

5959
/*******************************************************************************
60-
** Help work with MetaDataProducers.
60+
* Help work with MetaDataProducers.
61+
*
62+
* <p>Note that all of the public methods here use {@code sortMetaDataProducers},
63+
* which by default will sort by the sortOrder specified in the producer objects,
64+
* then by the type that they return, and finally by their class simple name, then
65+
* full name.</p>
66+
*
67+
* <p>But - this sorting by class names was added in 0.40.0. Prior to that,
68+
* there was no tie breaker on the sort. To restore the previous behavior, set
69+
* the system property {@code qqq.MetaDataProducerHelper.disableNameTiebreaker}
70+
* to {@code true}.</p>
71+
*
6172
*******************************************************************************/
6273
public class MetaDataProducerHelper
6374
{
@@ -66,10 +77,12 @@ public class MetaDataProducerHelper
6677
private static Map<Class<?>, Integer> comparatorValuesByType = new HashMap<>();
6778
private static Integer defaultComparatorValue;
6879

80+
private static boolean didLogAboutDisabledTiebreaker = false;
81+
6982
static
7083
{
7184
////////////////////////////////////////////////////////////////////////////////////////
72-
// define how we break ties in sort-order based on the meta-dta type. e.g., do apps //
85+
// define how we break ties in sort-order based on the meta-data type. e.g., do apps //
7386
// after all other types (as apps often try to get other types from the instance) //
7487
// also - do backends earlier than others (e.g., tables may expect backends to exist) //
7588
// any types not in the map get the default value. //
@@ -111,6 +124,7 @@ public static void processAllMetaDataProducersInPackage(QInstance instance, Stri
111124
}
112125

113126

127+
114128
/***************************************************************************
115129
**
116130
***************************************************************************/
@@ -190,11 +204,18 @@ public static List<MetaDataProducerInterface<?>> findProducers(String packageNam
190204

191205

192206
/***************************************************************************
193-
**
207+
* sort a list of producers: First by the sortOrder specified in the producer objects.
208+
* Second based on their types, so kinds that depend on other kinds come later.
209+
* Third, by class simple name, and 4th by full class name, to give stable ordering
210+
* (unless opt'ed out by system property).
211+
*
212+
* <p>Note, we use simple name before full name, in case, for some reason, you
213+
* wanted to control the sort by naming your classes a certain way - easier
214+
* to change Simple names than packages.</p>
194215
***************************************************************************/
195216
public static void sortMetaDataProducers(List<MetaDataProducerInterface<?>> producers)
196217
{
197-
producers.sort(Comparator
218+
Comparator<MetaDataProducerInterface<?>> comparator = Comparator
198219
.comparing((MetaDataProducerInterface<?> p) -> p.getSortOrder())
199220
.thenComparing((MetaDataProducerInterface<?> p) ->
200221
{
@@ -207,7 +228,32 @@ public static void sortMetaDataProducers(List<MetaDataProducerInterface<?>> prod
207228
{
208229
return (0);
209230
}
210-
}));
231+
});
232+
233+
//////////////////////////////////////////////////////////////////////////
234+
// read this system property each time this method is called. //
235+
// normally we might read it once into a static var - but, e.g., tests //
236+
// might need to change the value over time. and this isn't a hot code //
237+
// path at all, so we can bear the cost each time we're called in here. //
238+
//////////////////////////////////////////////////////////////////////////
239+
String propertyName = "qqq.MetaDataProducerHelper.disableNameTiebreaker";
240+
boolean disableNameTiebreaker = Boolean.getBoolean(propertyName);
241+
if(disableNameTiebreaker)
242+
{
243+
if(!didLogAboutDisabledTiebreaker)
244+
{
245+
didLogAboutDisabledTiebreaker = true;
246+
LOG.warn("Name tiebreaker is disabled for MetaDataProducerHelper.sortMetaDataProducers. This is legacy behavior, activated by system property [" + propertyName + "], which may be removed in a future release.");
247+
}
248+
}
249+
else
250+
{
251+
comparator = comparator
252+
.thenComparing(p -> p.getClass().getSimpleName())
253+
.thenComparing(p -> p.getClass().getName());
254+
}
255+
256+
producers.sort(comparator);
211257
}
212258

213259

@@ -387,8 +433,8 @@ private static MetaDataProducerInterface<?> processChildRecordListWidget(Class<?
387433
String parentTableName = getTableNameStaticFieldValue(sourceClass);
388434
String childTableName = getTableNameStaticFieldValue(childEntityClass);
389435

390-
ChildRecordListWidget childRecordListWidget = childTable.childRecordListWidget();
391-
ChildRecordListWidgetFromRecordEntityGenericMetaDataProducer producer = new ChildRecordListWidgetFromRecordEntityGenericMetaDataProducer(childTableName, parentTableName, childRecordListWidget);
436+
ChildRecordListWidget childRecordListWidget = childTable.childRecordListWidget();
437+
ChildRecordListWidgetFromRecordEntityGenericMetaDataProducer producer = new ChildRecordListWidgetFromRecordEntityGenericMetaDataProducer(childTableName, parentTableName, childRecordListWidget);
392438
producer.setSourceClass(sourceClass);
393439
return producer;
394440
}

qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/model/metadata/MetaDataProducerHelperTest.java

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
package com.kingsrook.qqq.backend.core.model.metadata;
2323

2424

25+
import java.util.ArrayList;
26+
import java.util.List;
2527
import com.kingsrook.qqq.backend.core.exceptions.QException;
2628
import com.kingsrook.qqq.backend.core.model.dashboard.widgets.WidgetType;
2729
import com.kingsrook.qqq.backend.core.model.metadata.dashboard.QWidgetMetaDataInterface;
@@ -40,6 +42,7 @@
4042
import com.kingsrook.qqq.backend.core.model.metadata.producers.TestNoInterfacesExtendsObject;
4143
import com.kingsrook.qqq.backend.core.model.metadata.producers.TestNoValidConstructorMetaDataProducer;
4244
import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData;
45+
import org.junit.jupiter.api.AfterEach;
4346
import org.junit.jupiter.api.Test;
4447
import static org.junit.jupiter.api.Assertions.assertEquals;
4548
import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -52,6 +55,20 @@
5255
*******************************************************************************/
5356
class MetaDataProducerHelperTest
5457
{
58+
private static final String DISABLE_NAME_TIEBREAKER_PROPERTY = "qqq.MetaDataProducerHelper.disableNameTiebreaker";
59+
60+
61+
62+
/***************************************************************************
63+
*
64+
***************************************************************************/
65+
@AfterEach
66+
void afterEach()
67+
{
68+
System.clearProperty(DISABLE_NAME_TIEBREAKER_PROPERTY);
69+
}
70+
71+
5572

5673
/*******************************************************************************
5774
**
@@ -122,4 +139,180 @@ void test() throws QException
122139

123140
}
124141

142+
143+
144+
/*******************************************************************************
145+
** Test that producers with the same sortOrder and type are sorted by class
146+
** simple name (alphabetically), then by full class name, for deterministic ordering.
147+
*******************************************************************************/
148+
@Test
149+
void testSortByClassSimpleNameThenFullName()
150+
{
151+
///////////////////////////////////////////////////////////////////////////
152+
// create producers with same sortOrder - they should sort by class name //
153+
///////////////////////////////////////////////////////////////////////////
154+
MetaDataProducerInterface<?> producerZ = new TestProducerZebra();
155+
MetaDataProducerInterface<?> producerA = new TestProducerAlpha();
156+
MetaDataProducerInterface<?> producerM = new TestProducerMango();
157+
158+
///////////////////////////////////////////////////////
159+
// put them in an "unsorted" order and sort the list //
160+
///////////////////////////////////////////////////////
161+
List<MetaDataProducerInterface<?>> producers = new ArrayList<>(List.of(producerZ, producerA, producerM));
162+
MetaDataProducerHelper.sortMetaDataProducers(producers);
163+
164+
//////////////////////////////////////////////////////////////
165+
// verify they are now sorted alphabetically by simple name //
166+
//////////////////////////////////////////////////////////////
167+
assertEquals("TestProducerAlpha", producers.get(0).getClass().getSimpleName());
168+
assertEquals("TestProducerMango", producers.get(1).getClass().getSimpleName());
169+
assertEquals("TestProducerZebra", producers.get(2).getClass().getSimpleName());
170+
}
171+
172+
173+
174+
/*******************************************************************************
175+
** Test that the system property disables the class name tiebreaker, restoring
176+
** the previous (undefined) behavior.
177+
*******************************************************************************/
178+
@Test
179+
void testDisableNameTiebreakerSystemProperty()
180+
{
181+
System.setProperty(DISABLE_NAME_TIEBREAKER_PROPERTY, "true");
182+
183+
MetaDataProducerInterface<?> producerZ = new TestProducerZebra();
184+
MetaDataProducerInterface<?> producerA = new TestProducerAlpha();
185+
186+
/////////////////////////////////////////////////////////////////////////////
187+
// with tiebreaker disabled, order should be based on insertion order //
188+
// (since sortOrder and type are equal, and no further comparator applied) //
189+
/////////////////////////////////////////////////////////////////////////////
190+
List<MetaDataProducerInterface<?>> producers = new ArrayList<>(List.of(producerZ, producerA));
191+
MetaDataProducerHelper.sortMetaDataProducers(producers);
192+
193+
//////////////////////////////////////////////////////////////////////////////
194+
// the order should remain as inserted (Z, A) since there's no tie-breaker. //
195+
// note: this relies on stable sort behavior in Java //
196+
//////////////////////////////////////////////////////////////////////////////
197+
assertEquals("TestProducerZebra", producers.get(0).getClass().getSimpleName());
198+
assertEquals("TestProducerAlpha", producers.get(1).getClass().getSimpleName());
199+
}
200+
201+
202+
203+
/*******************************************************************************
204+
** Test that sortOrder still takes precedence over class name.
205+
*******************************************************************************/
206+
@Test
207+
void testSortOrderTakesPrecedenceOverClassName()
208+
{
209+
///////////////////////////////////////////////////////////////////////////
210+
// producerZ has lower sortOrder, so should come first despite "Z" > "A" //
211+
///////////////////////////////////////////////////////////////////////////
212+
MetaDataProducerInterface<?> producerZ = new TestProducerZebra()
213+
{
214+
@Override
215+
public int getSortOrder()
216+
{
217+
return 100;
218+
}
219+
};
220+
MetaDataProducerInterface<?> producerA = new TestProducerAlpha()
221+
{
222+
@Override
223+
public int getSortOrder()
224+
{
225+
return 200;
226+
}
227+
};
228+
229+
List<MetaDataProducerInterface<?>> producers = new ArrayList<>(List.of(producerA, producerZ));
230+
MetaDataProducerHelper.sortMetaDataProducers(producers);
231+
232+
//////////////////////////////////////////////////////////////
233+
// Z should come first because it has lower sortOrder (100) //
234+
//////////////////////////////////////////////////////////////
235+
assertEquals(100, producers.get(0).getSortOrder());
236+
assertEquals(200, producers.get(1).getSortOrder());
237+
}
238+
239+
240+
241+
/*******************************************************************************
242+
** Test that when simple names are equal, full class name is used as tiebreaker.
243+
** This tests the scenario mentioned in the review: com.foo.MyProducer vs com.bar.MyProducer
244+
*******************************************************************************/
245+
@Test
246+
void testFullClassNameTiebreakerWhenSimpleNamesMatch()
247+
{
248+
/////////////////////////////////////////////////////////////////////////////////
249+
// create two producers from different inner classes that have the same simple //
250+
// name pattern (anonymous classes extending the same base) //
251+
// We'll use the outer class structure to create predictable full names //
252+
/////////////////////////////////////////////////////////////////////////////////
253+
MetaDataProducerInterface<?> producerFromAlpha = new TestProducerAlpha() {};
254+
MetaDataProducerInterface<?> producerFromZebra = new TestProducerZebra() {};
255+
256+
//////////////////////////////////////////////////////////////////////////////
257+
// both are anonymous classes, so their simple names will be empty strings. //
258+
// the full name will include the outer class and a number suffix. //
259+
// this exercises the full-name tiebreaker when simple names are equal. //
260+
//////////////////////////////////////////////////////////////////////////////
261+
assertEquals(producerFromAlpha.getClass().getSimpleName(), producerFromZebra.getClass().getSimpleName());
262+
263+
List<MetaDataProducerInterface<?>> producers = new ArrayList<>(List.of(producerFromZebra, producerFromAlpha));
264+
MetaDataProducerHelper.sortMetaDataProducers(producers);
265+
266+
/////////////////////////////////////////////////////////////////////////////////////
267+
// after sorting, they should be in a deterministic order based on full class name //
268+
// the key assertion is that sorting is stable and deterministic //
269+
/////////////////////////////////////////////////////////////////////////////////////
270+
String firstName = producers.get(0).getClass().getName();
271+
String secondName = producers.get(1).getClass().getName();
272+
assertTrue(firstName.compareTo(secondName) <= 0,
273+
"Expected first producer's full name [" + firstName + "] to sort before or equal to second [" + secondName + "]");
274+
}
275+
276+
277+
278+
/***************************************************************************
279+
* Test producer class - named to sort first alphabetically
280+
***************************************************************************/
281+
private static class TestProducerAlpha implements MetaDataProducerInterface<QTableMetaData>
282+
{
283+
@Override
284+
public QTableMetaData produce(QInstance qInstance)
285+
{
286+
return new QTableMetaData().withName("alpha");
287+
}
288+
}
289+
290+
291+
292+
/***************************************************************************
293+
* Test producer class - named to sort in the middle alphabetically
294+
***************************************************************************/
295+
private static class TestProducerMango implements MetaDataProducerInterface<QTableMetaData>
296+
{
297+
@Override
298+
public QTableMetaData produce(QInstance qInstance)
299+
{
300+
return new QTableMetaData().withName("mango");
301+
}
302+
}
303+
304+
305+
306+
/***************************************************************************
307+
* Test producer class - named to sort last alphabetically
308+
***************************************************************************/
309+
private static class TestProducerZebra implements MetaDataProducerInterface<QTableMetaData>
310+
{
311+
@Override
312+
public QTableMetaData produce(QInstance qInstance)
313+
{
314+
return new QTableMetaData().withName("zebra");
315+
}
316+
}
317+
125318
}

0 commit comments

Comments
 (0)